From 82dc7c1217667186a39a365c693b5738765a387a Mon Sep 17 00:00:00 2001 From: Ihnatus Date: Mon, 6 Jun 2022 21:44:52 +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 the game. Apply the split where it's appropriate, plumbing some memory issues. See OSDN#44738. Signed-off-by: Ihnatus --- server/diplomats.c | 55 ++++++------ server/scripting/api_server_edit.c | 47 +++++----- server/unithand.c | 13 +-- server/unittools.c | 132 ++++++++++++++++++++--------- server/unittools.h | 6 ++ 5 files changed, 161 insertions(+), 92 deletions(-) diff --git a/server/diplomats.c b/server/diplomats.c index d1ce80fb1c..6b2db4c019 100644 --- a/server/diplomats.c +++ b/server/diplomats.c @@ -633,7 +633,7 @@ bool diplomat_bribe(struct player *pplayer, struct unit *pdiplomat, int diplomat_id = pdiplomat->id; const struct unit_type *act_utype; struct city *pcity; - bool bounce; + bool bounce = FALSE; /* Fetch target unit's player. Sanity checks. */ fc_assert_ret_val(pvictim, FALSE); @@ -690,37 +690,42 @@ bool diplomat_bribe(struct player *pplayer, struct unit *pdiplomat, log_debug("bribe-unit: succeeded"); victim_tile = unit_tile(pvictim); + pcity = tile_city(victim_tile); + /* 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, 100)) { - 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, 100)) { + 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. */ + 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! */ + /* FIXME: Due to script effects, may get in debt here... */ pplayer->economic.gold -= bribe_cost; /* This may cause a diplomatic incident */ diff --git a/server/scripting/api_server_edit.c b/server/scripting/api_server_edit.c index 97d7bbde22..11579c7f78 100644 --- a/server/scripting/api_server_edit.c +++ b/server/scripting/api_server_edit.c @@ -146,6 +146,8 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, { 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); @@ -160,16 +162,26 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, 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); - if (!ret) { + if (!can_unit_load(punit, ptransport)) { + unit_virtual_destroy(punit); luascript_log(fcl, LOG_ERROR, "create_unit_full: '%s' cannot transport " "'%s' here", utype_rule_name(unit_type_get(ptransport)), @@ -177,27 +189,16 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, return NULL; } } else if (!can_exist_at_tile(&(wld.map), 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 8ba3330634..d1e5d15abe 100644 --- a/server/unithand.c +++ b/server/unithand.c @@ -337,9 +337,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, @@ -350,7 +351,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, @@ -368,11 +371,11 @@ static bool do_capture_units(struct player *pplayer, action_consequence_success(paction, pplayer, act_utype, 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; unit_did_action(punit); unit_forget_last_activity(punit); diff --git a/server/unittools.c b/server/unittools.c index ea96e3f205..b01d02e0da 100644 --- a/server/unittools.c +++ b/server/unittools.c @@ -1627,26 +1627,53 @@ 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 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, const struct unit_type *type, int veteran_level, int homecity_id, int moves_left, int hp_left, struct unit *ptrans) { - struct unit *punit = unit_virtual_create(pplayer, NULL, type, veteran_level); - struct city *pcity; + 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; - /* Register unit */ - punit->id = identity_number(); - idex_register_unit(&wld, punit); + 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; +} + +/**********************************************************************//** + 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_virtual(struct player *pplayer, struct tile *ptile, + const struct unit_type *type, + int veteran_level, int homecity_id, + int moves_left, int hp_left) +{ + 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 { @@ -1660,28 +1687,56 @@ 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(&wld, 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(&(wld.map), punit, ptile), NULL); + } + if (!unit_transport_get(punit)) { + fc_assert_ret_val(can_unit_exist_at_tile(&(wld.map), 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); + /* update unit upkeep */ + city_units_upkeep(pcity); /* Refresh the unit's homecity. */ city_refresh(pcity); send_city_info(pplayer, pcity); @@ -1691,12 +1746,8 @@ struct unit *create_unit_full(struct player *pplayer, struct tile *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(); @@ -1706,7 +1757,7 @@ 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 punit; + return TRUE; } /**********************************************************************//** @@ -2124,20 +2175,21 @@ static bool try_to_save_unit(struct unit *punit, const 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; @@ -2147,21 +2199,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 dee05697bc..547d507864 100644 --- a/server/unittools.h +++ b/server/unittools.h @@ -128,6 +128,12 @@ struct unit *create_unit_full(struct player *pplayer, struct tile *ptile, const 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, + const 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