Author: jtn Date: Mon Jun 13 10:10:47 2016 New Revision: 32844 URL: http://svn.gna.org/viewcvs/freeciv?rev=32844&view=rev Log: Do not re-home homeless units when transferred with a city.
See gna bug #24746. Modified: trunk/server/citytools.c trunk/server/unithand.c trunk/server/unithand.h Modified: trunk/server/citytools.c URL: http://svn.gna.org/viewcvs/freeciv/trunk/server/citytools.c?rev=32844&r1=32843&r2=32844&view=diff ============================================================================== --- trunk/server/citytools.c (original) +++ trunk/server/citytools.c Mon Jun 13 10:10:47 2016 @@ -571,21 +571,26 @@ } /********************************************************************* - Change home city of a unit with verbose output. + Change player that owns a unit and, if appropriate, its home city, + with verbose output. + If 'rehome' is not set, only change the player which owns the unit + (the new owner is new_pcity's owner). Otherwise the new unit will be + given a homecity, even if it was homeless before. ***********************************************************************/ static void transfer_unit(struct unit *punit, struct city *tocity, - bool verbose) + bool rehome, bool verbose) { struct player *from_player = unit_owner(punit); struct player *to_player = city_owner(tocity); - /* Transfering a dying GameLoss unit as part of the loot for + /* Transferring a dying GameLoss unit as part of the loot for * killing it caused gna bug #23676. */ fc_assert_ret_msg(!punit->server.dying, "Tried to transfer the dying unit %d.", punit->id); if (from_player == to_player) { + fc_assert_ret(rehome); log_verbose("Changed homecity of %s %s to %s", nation_rule_name(nation_of_player(from_player)), unit_rule_name(punit), @@ -681,21 +686,22 @@ maybe_make_contact(utile, to_player); } - unit_change_homecity_handling(punit, tocity); + unit_change_homecity_handling(punit, tocity, rehome); } /********************************************************************* - Units in a bought city are transferred to the new owner, units + When a city is transferred (bought, incited, disbanded, civil war): + Units in a transferred city are transferred to the new owner; units supported by the city, but held in other cities are updated to reflect those cities as their new homecity. Units supported - by the bought city, that are not in a city square may be deleted. + by the transferred city that are not in a city tile may be deleted. - Kris Bubendorfer <kris.bubendor...@mcs.vuw.ac.nz> -pplayer: The player recieving the units if they are not disbanded and +pplayer: The player receiving the units if they are not disbanded and are not in a city pvictim: The owner of the city the units are transferred from. -units: A list of units to be transferred, typically a cities unit list. +units: A list of units to be transferred, typically a city's unit list. pcity: Default city the units are transferred to. exclude_city: The units cannot be transferred to this city. kill_outside: Units outside this range are deleted. -1 means no units @@ -726,20 +732,28 @@ * the dying unit isn't a good idea. The remaining death handling * code will try to read from it. * - * Transfering a dying GameLoss unit as part of the loot for + * Transferring a dying GameLoss unit as part of the loot for * killing it caused gna bug #23676. */ continue; } /* Don't transfer units already owned by new city-owner --wegge */ if (unit_owner(vunit) == pvictim) { + /* Determine whether unit was homeless. If it was, we don't give + * it a homecity, only change ownership. + * We have to search the transferred city's former units because + * the unit may have been made only temporarily homeless during + * city transfer. */ + bool homeless = (vunit->homecity == 0) + && !unit_list_search(units, vunit); + /* vunit may die during transfer_unit(). * unit_list_remove() is still safe using vunit pointer, as * pointer is not used for dereferencing, only as value. * Not sure if it would be safe to unlink first and transfer only * after that. Not sure if it is correct to unlink at all in * some cases, depending which list 'units' points to. */ - transfer_unit(vunit, pcity, verbose); + transfer_unit(vunit, pcity, !homeless, verbose); unit_list_remove(units, vunit); } else if (!pplayers_allied(pplayer, unit_owner(vunit))) { /* the owner of vunit is allied to pvictim but not to pplayer */ @@ -769,13 +783,14 @@ if (new_home_city && new_home_city != exclude_city && city_owner(new_home_city) == unit_owner(vunit)) { /* unit is in another city: make that the new homecity, - unless that city is actually the same city (happens if disbanding) */ - transfer_unit(vunit, new_home_city, verbose); + * unless that city is actually the same city (happens if disbanding) + * Unit had a homecity since it was supported, so rehome. */ + transfer_unit(vunit, new_home_city, TRUE, verbose); } else if ((kill_outside == -1 || real_map_distance(unit_tile(vunit), ptile) <= kill_outside) && saved_id) { /* else transfer to specified city. */ - transfer_unit(vunit, pcity, verbose); + transfer_unit(vunit, pcity, TRUE, verbose); if (unit_tile(vunit) == ptile && !pplayers_allied(pplayer, pvictim)) { /* Unit is inside city being transferred, bounce it */ bounce_unit(vunit, TRUE); @@ -1619,7 +1634,7 @@ && new_home_city != pcity && city_owner(new_home_city) == powner && !punit->server.dying) { - transfer_unit(punit, new_home_city, TRUE); + transfer_unit(punit, new_home_city, TRUE, TRUE); } } unit_list_iterate_safe_end; Modified: trunk/server/unithand.c URL: http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.c?rev=32844&r1=32843&r2=32844&view=diff ============================================================================== --- trunk/server/unithand.c (original) +++ trunk/server/unithand.c Mon Jun 13 10:10:47 2016 @@ -2242,10 +2242,14 @@ } /************************************************************************** - Transfer a unit from one homecity to another. This new homecity must - be valid for this unit. + Transfer a unit from one city (and possibly player) to another. + If 'rehome' is not set, only change the player which owns the unit + (the new owner is new_pcity's owner). Otherwise the new unit will be + given a homecity, even if it was homeless before. + This new homecity must be valid for this unit. **************************************************************************/ -void unit_change_homecity_handling(struct unit *punit, struct city *new_pcity) +void unit_change_homecity_handling(struct unit *punit, struct city *new_pcity, + bool rehome) { struct city *old_pcity = game_city_by_number(punit->homecity); struct player *old_owner = unit_owner(punit); @@ -2256,6 +2260,10 @@ * be used that way. */ fc_assert_ret(new_pcity != old_pcity); + /* If 'rehome' is not set, this function should only be used to change + * which player owns the unit */ + fc_assert_ret(rehome || new_owner != old_owner); + if (old_owner != new_owner) { struct city *pcity = tile_city(punit->tile); @@ -2267,11 +2275,11 @@ if (pcity != NULL && !can_player_see_units_in_city(old_owner, pcity)) { - /* Special case when city is being transfered. At this point city + /* Special case when city is being transferred. At this point city * itself has changed owner, so it's enemy city now that old owner - * cannot see inside. All the normal methods of removing transfered + * cannot see inside. All the normal methods of removing transferred * unit from previous owner's client think that there's no need to - * remove unit as client should't have it in first place. */ + * remove unit as client shouldn't have it in first place. */ unit_goes_out_of_sight(old_owner, punit); } @@ -2289,23 +2297,27 @@ unit_refresh_vision(punit); } - /* Remove from old city first and add to new city only after that. - * This is more robust in case old_city == new_city (currently - * prohibited by fc_assert in the beginning of the function). - */ - if (old_pcity) { - /* Even if unit is dead, we have to unlink unit pointer (punit). */ - unit_list_remove(old_pcity->units_supported, punit); + if (rehome) { + fc_assert(!unit_has_type_flag(punit, UTYF_NOHOME)); + + /* Remove from old city first and add to new city only after that. + * This is more robust in case old_city == new_city (currently + * prohibited by fc_assert in the beginning of the function). + */ + if (old_pcity) { + /* Even if unit is dead, we have to unlink unit pointer (punit). */ + unit_list_remove(old_pcity->units_supported, punit); + /* update unit upkeep */ + city_units_upkeep(old_pcity); + } + + unit_list_prepend(new_pcity->units_supported, punit); + /* update unit upkeep */ - city_units_upkeep(old_pcity); - } - - unit_list_prepend(new_pcity->units_supported, punit); - - /* update unit upkeep */ - city_units_upkeep(new_pcity); - - punit->homecity = new_pcity->id; + city_units_upkeep(new_pcity); + + punit->homecity = new_pcity->id; + } if (!can_unit_continue_current_activity(punit)) { /* This is mainly for cases where unit owner changes to one not knowing @@ -2336,7 +2348,7 @@ static bool do_unit_change_homecity(struct unit *punit, struct city *pcity) { - unit_change_homecity_handling(punit, pcity); + unit_change_homecity_handling(punit, pcity, TRUE); return punit->homecity == pcity->id; } Modified: trunk/server/unithand.h URL: http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.h?rev=32844&r1=32843&r2=32844&view=diff ============================================================================== --- trunk/server/unithand.h (original) +++ trunk/server/unithand.h Mon Jun 13 10:10:47 2016 @@ -22,7 +22,8 @@ void unit_activity_handling_targeted(struct unit *punit, enum unit_activity new_activity, struct extra_type **new_target); -void unit_change_homecity_handling(struct unit *punit, struct city *new_pcity); +void unit_change_homecity_handling(struct unit *punit, struct city *new_pcity, + bool rehome); bool unit_move_handling(struct unit *punit, struct tile *pdesttile, bool igzoc, bool move_diplomat_city, _______________________________________________ Freeciv-commits mailing list Freeciv-commits@gna.org https://mail.gna.org/listinfo/freeciv-commits