From 7853adf83f6dd400945407f518e18a8d793e63f3 Mon Sep 17 00:00:00 2001 From: Ihnatus Date: Mon, 6 Jun 2022 19:44:39 +0300 Subject: [PATCH] Split create_unit_full() in two functions: create_unit_virtual() makes a virtual unit with some initialized values that may be furtherly modified, place_unit() puts a prepared virtual unit into a game. Apply the split where it's appropriate, plumbing some memory issues and fixing comments and formatting in process. See OSDN#44738. Signed-off-by: Ihnatus --- common/unit.h | 2 +- server/diplomats.c | 54 +++++------ server/scripting/api_server_edit.c | 47 +++++----- server/unithand.c | 13 +-- server/unittools.c | 139 ++++++++++++++++++++--------- server/unittools.h | 16 ++-- 6 files changed, 171 insertions(+), 100 deletions(-) diff --git a/common/unit.h b/common/unit.h index e3f34b0792..acee2c855b 100644 --- a/common/unit.h +++ b/common/unit.h @@ -368,7 +368,7 @@ bool is_tile_activity(enum unit_activity activity); struct unit *unit_virtual_create(struct player *pplayer, struct city *pcity, struct unit_type *punittype, - int veteran_level); + int veteran_level); void unit_virtual_destroy(struct unit *punit); bool unit_is_virtual(const struct unit *punit); void free_unit_orders(struct unit *punit); diff --git a/server/diplomats.c b/server/diplomats.c index 5e389120e6..eb4c34c40a 100644 --- a/server/diplomats.c +++ b/server/diplomats.c @@ -467,7 +467,7 @@ bool diplomat_bribe(struct player *pplayer, struct unit *pdiplomat, int bribe_cost; int diplomat_id = pdiplomat->id; struct city *pcity; - bool bounce; + bool bounce = FALSE; /* Fetch target unit's player. Sanity checks. */ fc_assert_ret_val(pvictim, FALSE); @@ -521,34 +521,38 @@ bool diplomat_bribe(struct player *pplayer, struct unit *pdiplomat, log_debug("bribe-unit: succeeded"); victim_tile = unit_tile(pvictim); + /* Fallback value for notifications if scripts destroy the new unit */ + sz_strlcpy(victim_link, utype_name_translation(unit_type_get(pvictim))); pvictim = unit_change_owner(pvictim, pplayer, pdiplomat->homecity, ULR_BRIBED); - /* N.B.: unit_link always returns the same pointer. As unit_change_owner() - * currently remove the old unit and replace by a new one (with a new id), - * we want to make link to the new unit. */ - sz_strlcpy(victim_link, unit_link(pvictim)); + if (pvictim) { + /* N.B.: unit_link always returns the same pointer. + * As unit_change_owner() currently remove the old unit and replace by + * a new one (with a new id), we want to make link to the new unit. */ + sz_strlcpy(victim_link, unit_link(pvictim)); - /* Notify everybody involved. */ - notify_player(pplayer, victim_tile, E_MY_DIPLOMAT_BRIBE, ftc_server, - /* TRANS: ... */ - _("Your %s succeeded in bribing the %s."), - unit_link(pdiplomat), victim_link); - if (maybe_make_veteran(pdiplomat)) { - notify_unit_experience(pdiplomat); - } - notify_player(uplayer, victim_tile, E_ENEMY_DIPLOMAT_BRIBE, ftc_server, - /* TRANS: ... */ - _("Your %s was bribed by the %s."), - victim_link, nation_plural_for_player(pplayer)); - - /* The unit may have been on a tile shared with a city or a unit - * it no longer can share a tile with. */ - pcity = tile_city(unit_tile(pvictim)); - bounce = ((NULL != pcity && !pplayers_allied(city_owner(pcity), pplayer)) - || 1 < unit_list_size(unit_tile(pvictim)->units)); - if (bounce) { - bounce_unit(pvictim, TRUE); + /* Notify everybody involved. */ + notify_player(pplayer, victim_tile, E_MY_DIPLOMAT_BRIBE, ftc_server, + /* TRANS: ... */ + _("Your %s succeeded in bribing the %s."), + unit_link(pdiplomat), victim_link); + if (maybe_make_veteran(pdiplomat)) { + notify_unit_experience(pdiplomat); + } + notify_player(uplayer, victim_tile, E_ENEMY_DIPLOMAT_BRIBE, ftc_server, + /* TRANS: ... */ + _("Your %s was bribed by the %s."), + victim_link, nation_plural_for_player(pplayer)); + + /* The unit may have been on a tile shared with a city or a unit + * it no longer can share a tile with. */ + pcity = tile_city(unit_tile(pvictim)); + bounce = ((NULL != pcity && !pplayers_allied(city_owner(pcity), pplayer)) + || 1 < unit_list_size(unit_tile(pvictim)->units)); + if (bounce) { + bounce_unit(pvictim, TRUE); + } } /* This costs! */ diff --git a/server/scripting/api_server_edit.c b/server/scripting/api_server_edit.c index 56396c04b4..ff80fc8ef2 100644 --- a/server/scripting/api_server_edit.c +++ b/server/scripting/api_server_edit.c @@ -97,6 +97,8 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, Tile *ptile, { struct fc_lua *fcl; struct city *pcity; + struct unit *punit; + bool placed; LUASCRIPT_CHECK_STATE(L, NULL); LUASCRIPT_CHECK_ARG_NIL(L, pplayer, 2, Player, NULL); @@ -111,16 +113,28 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, Tile *ptile, return NULL; } + if (is_non_allied_unit_tile(ptile, pplayer)) { + luascript_log(fcl, LOG_ERROR, "create_unit_full: tile is occupied by " + "enemy unit"); + return NULL; + } + + pcity = tile_city(ptile); + if (pcity != NULL && !pplayers_allied(pplayer, city_owner(pcity))) { + luascript_log(fcl, LOG_ERROR, "create_unit_full: tile is occupied by " + "enemy city"); + return NULL; + } + + punit = create_unit_virtual(pplayer, ptile, ptype, veteran_level, + homecity ? homecity->id : 0, moves_left, + hp_left); if (ptransport) { /* Extensive check to see if transport and unit are compatible */ - int ret; - struct unit *pvirt = unit_virtual_create(pplayer, NULL, ptype, - veteran_level); - unit_tile_set(pvirt, ptile); - pvirt->homecity = homecity ? homecity->id : 0; - ret = can_unit_load(pvirt, ptransport); - unit_virtual_destroy(pvirt); + int ret = can_unit_load(punit, ptransport); + if (!ret) { + unit_virtual_destroy(punit); luascript_log(fcl, LOG_ERROR, "create_unit_full: '%s' cannot transport " "'%s' here", utype_rule_name(unit_type_get(ptransport)), @@ -128,27 +142,16 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, Tile *ptile, return NULL; } } else if (!can_exist_at_tile(ptype, ptile)) { + unit_virtual_destroy(punit); luascript_log(fcl, LOG_ERROR, "create_unit_full: '%s' cannot exist at " "tile", utype_rule_name(ptype)); return NULL; } - if (is_non_allied_unit_tile(ptile, pplayer)) { - luascript_log(fcl, LOG_ERROR, "create_unit_full: tile is occupied by " - "enemy unit"); - return NULL; - } - - pcity = tile_city(ptile); - if (pcity != NULL && !pplayers_allied(pplayer, city_owner(pcity))) { - luascript_log(fcl, LOG_ERROR, "create_unit_full: tile is occupied by " - "enemy city"); - return NULL; - } + placed = place_unit(punit, pplayer, homecity, ptransport); + fc_assert_action(placed, unit_virtual_destroy(punit); punit = NULL); - return create_unit_full(pplayer, ptile, ptype, veteran_level, - homecity ? homecity->id : 0, moves_left, - hp_left, ptransport); + return punit; } /***************************************************************************** diff --git a/server/unithand.c b/server/unithand.c index 34ae97e10a..d63e807912 100644 --- a/server/unithand.c +++ b/server/unithand.c @@ -315,9 +315,10 @@ static bool do_capture_units(struct player *pplayer, sz_strlcpy(capturer_link, unit_link(punit)); pcity = tile_city(pdesttile); - unit_list_iterate(pdesttile->units, to_capture) { + unit_list_iterate_safe(pdesttile->units, to_capture) { struct player *uplayer = unit_owner(to_capture); - const char *victim_link; + const char *victim_link + = utype_name_translation(unit_type_get(to_capture)); unit_owner(to_capture)->score.units_lost++; to_capture = unit_change_owner(to_capture, pplayer, @@ -328,7 +329,9 @@ static bool do_capture_units(struct player *pplayer, /* As unit_change_owner() currently remove the old unit and * replace by a new one (with a new id), we want to make link to * the new unit. */ - victim_link = unit_link(to_capture); + if (to_capture) { + victim_link = unit_link(to_capture); + } /* Notify players */ notify_player(pplayer, pdesttile, E_MY_DIPLOMAT_BRIBE, ftc_server, @@ -346,11 +349,11 @@ static bool do_capture_units(struct player *pplayer, action_consequence_success(paction, pplayer, uplayer, pdesttile, victim_link); - if (NULL != pcity) { + if (to_capture && NULL != pcity) { /* The captured unit is in a city. Bounce it. */ bounce_unit(to_capture, TRUE); } - } unit_list_iterate_end; + } unit_list_iterate_safe_end; /* Subtract movement point from capturer */ punit->moves_left -= SINGLE_MOVE; diff --git a/server/unittools.c b/server/unittools.c index 8d431f23fc..9e80c1057a 100644 --- a/server/unittools.c +++ b/server/unittools.c @@ -1584,27 +1584,27 @@ void unit_get_goods(struct unit *punit) } } -/************************************************************************** - Creates a unit, and set it's initial values, and put it into the right - lists. - If moves_left is less than zero, unit will get max moves. +/**********************************************************************//** + Creates a virtual unit, and sets its initial values, but does not + register the unit in any other data structures or set the vision. + If moves_left is less than zero, unit will get max moves; + otherwise, it will get the specified number of movement fragments + and will be considered moved. + If hp_left is zero or less, unit will get full hp. + homecity_id won't be set to units with "NoHome" flag. + ptile must be a valid tile (its livability for the unit is not checked) **************************************************************************/ -struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, - struct unit_type *type, int veteran_level, - int homecity_id, int moves_left, int hp_left, - struct unit *ptrans) +struct unit *create_unit_virtual(struct player *pplayer, struct tile *ptile, + struct unit_type *type, + int veteran_level, int homecity_id, + int moves_left, int hp_left) { - struct unit *punit = unit_virtual_create(pplayer, NULL, type, veteran_level); - struct city *pcity; - - /* Register unit */ - punit->id = identity_number(); - idex_register_unit(punit); + struct unit *punit; fc_assert_ret_val(ptile != NULL, NULL); + punit = unit_virtual_create(pplayer, NULL, type, veteran_level); unit_tile_set(punit, ptile); - pcity = game_city_by_number(homecity_id); if (utype_has_flag(type, UTYF_NOHOME)) { punit->homecity = 0; /* none */ } else { @@ -1618,42 +1618,67 @@ struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, if (moves_left >= 0) { /* Override default full MP */ + /* FIXME: there are valid situations when a unit have mp + * over its move rate. Here, keep the old behavior. */ punit->moves_left = MIN(moves_left, unit_move_rate(punit)); + punit->moved = TRUE; } + return punit; +} + +/**********************************************************************//** + Places a virtual unit into the game, assigning it an index, putting it + on the right lists and dispatching the information around. + The unit must have a tile (see create_unit_virtual()), + pcity and pplayer must be valid and accord to the unit's fields. + Units with flag "NoHome" won't be homed. + ptrans if not NULL must be a transporter on the same tile + the unit can freely load into. + Returns if the unit is placed (must be TRUE if input data are valid) +**************************************************************************/ +bool place_unit(struct unit *punit, struct player *pplayer, + struct city *pcity, struct unit *ptrans) +{ + struct tile *ptile; + + fc_assert_ret_val(pplayer, FALSE); + fc_assert_ret_val(punit, FALSE); + ptile = punit->tile; + fc_assert_ret_val(ptile, FALSE); + + /* Register unit */ + punit->id = identity_number(); + idex_register_unit(punit); if (ptrans) { /* Set transporter for unit. */ unit_transport_load_tp_status(punit, ptrans, FALSE); - } else { - fc_assert_ret_val(!ptile || can_unit_exist_at_tile(punit, ptile), NULL); + } + if (!unit_transport_get(punit)) { + fc_assert_ret_val(can_unit_exist_at_tile(punit, ptile), + FALSE); } - /* Assume that if moves_left < 0 then the unit is "fresh", - * and not moved; else the unit has had something happen - * to it (eg, bribed) which we treat as equivalent to moved. - * (Otherwise could pass moved arg too...) --dwp */ - punit->moved = (moves_left >= 0); + maybe_make_contact(ptile, pplayer); unit_list_prepend(pplayer->units, punit); unit_list_prepend(ptile->units, punit); - if (pcity && !utype_has_flag(type, UTYF_NOHOME)) { + if (pcity && !unit_has_type_flag(punit, UTYF_NOHOME)) { fc_assert(city_owner(pcity) == pplayer); unit_list_prepend(pcity->units_supported, punit); /* Refresh the unit's homecity. */ city_refresh(pcity); send_city_info(pplayer, pcity); + /* update unit upkeep */ + city_units_upkeep(pcity); } punit->server.vision = vision_new(pplayer, ptile); unit_refresh_vision(punit); send_unit_info(NULL, punit); - maybe_make_contact(ptile, unit_owner(punit)); wakeup_neighbor_sentries(punit); - /* update unit upkeep */ - city_units_upkeep(game_city_by_number(homecity_id)); - /* The unit may have changed the available tiles in nearby cities. */ city_map_update_tile_now(ptile); sync_cities(); @@ -1663,6 +1688,33 @@ struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, CALL_FUNC_EACH_AI(unit_created, punit); CALL_PLR_AI_FUNC(unit_got, pplayer, punit); + return TRUE; +} + +/************************************************************************** + Creates a unit, sets its initial values, and puts it into the right + lists. The unit must be placed to a valid tile or a loadable transport. + See create_unit_virtual() for the processing of moves_left and hp_left +**************************************************************************/ +struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, + struct unit_type *type, int veteran_level, + int homecity_id, int moves_left, int hp_left, + struct unit *ptrans) +{ + struct unit *punit + = create_unit_virtual(pplayer, ptile, type, veteran_level, + homecity_id, moves_left, hp_left); + struct city *pcity = game_city_by_number(homecity_id); + bool could_place; + + fc_assert_ret_val(punit, NULL); + could_place = place_unit(punit, pplayer, pcity, ptrans); + fc_assert(could_place); + if (!could_place) { + unit_virtual_destroy(punit); + punit = NULL; + } + return punit; } @@ -2081,20 +2133,21 @@ static bool try_to_save_unit(struct unit *punit, struct unit_type *pttype, /**************************************************************************** We don't really change owner of the unit, but create completely new unit as its copy. The new pointer to 'punit' is returned. + Returns NULL if the new unit was killed in process. ****************************************************************************/ struct unit *unit_change_owner(struct unit *punit, struct player *pplayer, int homecity, enum unit_loss_reason reason) { struct unit *gained_unit; + int id = IDENTITY_NUMBER_ZERO; fc_assert(!utype_player_already_has_this_unique(pplayer, unit_type_get(punit))); - /* Convert the unit to your cause. Fog is lifted in the create algorithm. */ - gained_unit = create_unit_full(pplayer, unit_tile(punit), - unit_type_get(punit), punit->veteran, - homecity, punit->moves_left, - punit->hp, NULL); + /* Convert the unit to your cause */ + gained_unit = create_unit_virtual(pplayer, unit_tile(punit), + unit_type_get(punit), punit->veteran, + homecity, punit->moves_left, punit->hp); /* Owner changes, nationality not. */ gained_unit->nationality = punit->nationality; @@ -2104,21 +2157,23 @@ struct unit *unit_change_owner(struct unit *punit, struct player *pplayer, gained_unit->paradropped = punit->paradropped; gained_unit->server.birth_turn = punit->server.birth_turn; - send_unit_info(NULL, gained_unit); - - /* update unit upkeep in the homecity of the victim */ - if (punit->homecity > 0) { - /* update unit upkeep */ - city_units_upkeep(game_city_by_number(punit->homecity)); - } - /* update unit upkeep in the new homecity */ - if (homecity > 0) { - city_units_upkeep(game_city_by_number(homecity)); + /* This call implies that the unit is not transported. + * Fog is lifted in the placing algorithm. */ + if (!place_unit(gained_unit, pplayer, + game_city_by_number(homecity), NULL)) { + fc_assert(FALSE); + unit_virtual_destroy(gained_unit); + } else { + send_unit_info(NULL, gained_unit); + id = gained_unit->id; } /* Be sure to wipe the converted unit! */ wipe_unit(punit, reason, NULL); + if (!unit_is_alive(id)) { + gained_unit = NULL; + } return gained_unit; /* Returns the replacement. */ } diff --git a/server/unittools.h b/server/unittools.h index 1161847c2e..ee1fb464b7 100644 --- a/server/unittools.h +++ b/server/unittools.h @@ -121,12 +121,18 @@ void unit_forget_last_activity(struct unit *punit); void transform_unit(struct unit *punit, struct unit_type *to_unit, int vet_loss); struct unit *create_unit(struct player *pplayer, struct tile *ptile, - struct unit_type *punittype, - int veteran_level, int homecity_id, int moves_left); + struct unit_type *punittype, + int veteran_level, int homecity_id, int moves_left); struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, - struct unit_type *punittype, int veteran_level, - int homecity_id, int moves_left, int hp_left, - struct unit *ptrans); + struct unit_type *punittype, int veteran_level, + int homecity_id, int moves_left, int hp_left, + struct unit *ptrans); +struct unit *create_unit_virtual(struct player *pplayer, struct tile *ptile, + struct unit_type *type, + int veteran_level, int homecity_id, + int moves_left, int hp_left); +bool place_unit(struct unit *punit, struct player *pplayer, + struct city *pcity, struct unit *ptrans); void wipe_unit(struct unit *punit, enum unit_loss_reason reason, struct player *killer); void kill_unit(struct unit *pkiller, struct unit *punit, bool vet); -- 2.34.1