cghislai has proposed merging lp:~widelands-dev/widelands/bug1205010 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1205010 in widelands: "segfault on dismantle enemy building"
https://bugs.launchpad.net/widelands/+bug/1205010
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug1205010/+merge/177137
This fixes the linked bug. When a building is conquered, it goes through all
Building_Descr in the former_buildings vector (the previous enhancements), add
the suffix and reload from the conquering tribe. This allows correct
computation of returned wares on dismantle and prevent the crash.
The force_building function signature changed and I split the construction site
case in a force_csite function. I updated the code wherever i could grep its
use.
I tested conquering building from the same tribe or another one. In the former
case, in case of enhanced building, the correct amount of ware was computed for
dismantling.
--
https://code.launchpad.net/~widelands-dev/widelands/bug1205010/+merge/177137
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug1205010 into lp:widelands.
=== modified file 'src/logic/militarysite.cc'
--- src/logic/militarysite.cc 2013-07-24 09:19:24 +0000
+++ src/logic/militarysite.cc 2013-07-26 11:30:40 +0000
@@ -864,23 +864,30 @@
// the old location.
Player * enemyplayer = enemy.get_owner();
const Tribe_Descr & enemytribe = enemyplayer->tribe();
- std::string bldname = name();
- // Has this building already a suffix? == conquered building?
- std::string::size_type const dot = bldname.rfind('.');
- if (dot >= bldname.size()) {
- // Add suffix, if the new owner uses another tribe than we.
- if (enemytribe.name() != owner().tribe().name())
- bldname += "." + owner().tribe().name();
- } else if (enemytribe.name() == bldname.substr(dot + 1, bldname.size()))
- bldname = bldname.substr(0, dot);
- Building_Index bldi = enemytribe.safe_building_index(bldname.c_str());
+ // Add suffix to all descr in former buildings in cases
+ // the new owner comes from another tribe
+ Building_Descr::FormerBuildings former_buildings;
+ BOOST_FOREACH(const Building_Descr * old_descr, m_old_buildings) {
+ std::string bldname = old_descr->name();
+ // Has this building already a suffix? == conquered building?
+ std::string::size_type const dot = bldname.rfind('.');
+ if (dot >= bldname.size()) {
+ // Add suffix, if the new owner uses another tribe than we.
+ if (enemytribe.name() != owner().tribe().name())
+ bldname += "." + owner().tribe().name();
+ } else if (enemytribe.name() == bldname.substr(dot + 1, bldname.size()))
+ bldname = bldname.substr(0, dot);
+ Building_Index bldi = enemytribe.safe_building_index(bldname.c_str());
+ const Building_Descr * former_descr = enemytribe.get_building_descr(bldi);
+ former_buildings.push_back(former_descr);
+ }
// Now we destroy the old building before we place the new one.
set_defeating_player(enemy.owner().player_number());
schedule_destroy(game);
- enemyplayer->force_building(coords, bldi);
+ enemyplayer->force_building(coords, former_buildings);
BaseImmovable * const newimm = game.map()[coords].get_immovable();
upcast(MilitarySite, newsite, newimm);
newsite->reinit_after_conqueration(game);
=== modified file 'src/logic/player.cc'
--- src/logic/player.cc 2013-07-22 07:26:07 +0000
+++ src/logic/player.cc 2013-07-26 11:30:40 +0000
@@ -404,11 +404,8 @@
return Road::create(egbase(), start, end, path);
}
-
-Building & Player::force_building
- (Coords const location,
- Building_Index const idx,
- bool constructionsite)
+void Player::prepare_terrain_for_building
+ (Coords const location, const Building_Descr * descr)
{
Map & map = egbase().map();
FCoords c[4]; // Big buildings occupy 4 locations.
@@ -417,10 +414,9 @@
force_flag(c[1]);
if (BaseImmovable * const immovable = c[0].field->get_immovable())
immovable->remove(egbase());
- const Building_Descr & descr = *tribe().get_building_descr(idx);
{
size_t nr_locations = 1;
- if ((descr.get_size() & BUILDCAPS_SIZEMASK) == BUILDCAPS_BIG) {
+ if ((descr->get_size() & BUILDCAPS_SIZEMASK) == BUILDCAPS_BIG) {
nr_locations = 4;
map.get_trn(c[0], &c[1]);
map.get_tln(c[0], &c[2]);
@@ -437,12 +433,32 @@
immovable->remove(egbase());
}
}
-
- if (constructionsite)
- return egbase().warp_constructionsite(c[0], m_plnum, idx);
- else
- return descr.create (egbase(), *this, c[0], false);
-}
+}
+
+Building & Player::force_building
+ (Coords const location,
+ Building_Descr::FormerBuildings former_buildings)
+{
+ Map & map = egbase().map();
+ const Building_Descr* descr = former_buildings.back();
+ prepare_terrain_for_building(location, descr);
+ return
+ descr->create
+ (egbase(), *this, map.get_fcoords(location), false, false, former_buildings);
+}
+
+Building& Player::force_csite
+ (Coords const location, Building_Index b_idx,
+ Building_Descr::FormerBuildings former_buildings)
+{
+ Map & map = egbase().map();
+ const Building_Descr * descr = tribe().get_building_descr(b_idx);
+ prepare_terrain_for_building(location, descr);
+ return
+ egbase().warp_constructionsite
+ (map.get_fcoords(location), m_plnum, b_idx, false, former_buildings);
+}
+
/*
=== modified file 'src/logic/player.h'
--- src/logic/player.h 2013-07-08 03:35:09 +0000
+++ src/logic/player.h 2013-07-26 11:30:40 +0000
@@ -455,8 +455,11 @@
Road * build_road(const Path &); /// Build a road if it is allowed.
Building & force_building
(Coords,
+ Building_Descr::FormerBuildings);
+ Building & force_csite
+ (Coords,
Building_Index,
- bool = false);
+ Building_Descr::FormerBuildings = Building_Descr::FormerBuildings());
Building * build(Coords, Building_Index, bool = true);
void bulldoze(PlayerImmovable &, bool recurse = false);
void flagaction(Flag &);
@@ -572,6 +575,8 @@
void play_message_sound(const std::string & sender);
void _enhance_or_dismantle
(Building *, Building_Index const index_of_new_building = Building_Index::Null());
+ // Used in force_building / force_csite
+ void prepare_terrain_for_building(Coords location, const Building_Descr * descr);
private:
MessageQueue m_messages;
=== modified file 'src/logic/ship.cc'
--- src/logic/ship.cc 2013-07-21 08:27:10 +0000
+++ src/logic/ship.cc 2013-07-26 11:30:40 +0000
@@ -658,7 +658,8 @@
/// @note only called via player command
void Ship::exp_construct_port (Game &, const Coords& c) {
assert(m_expedition);
- m_economy->owner().force_building(c, m_economy->owner().tribe().safe_building_index("port"), true);
+ Building_Index port_idx = m_economy->owner().tribe().safe_building_index("port");
+ m_economy->owner().force_csite(c, port_idx);
m_ship_state = EXP_COLONIZING;
}
=== modified file 'src/scripting/lua_bases.cc'
--- src/scripting/lua_bases.cc 2012-06-08 22:33:16 +0000
+++ src/scripting/lua_bases.cc 2013-07-26 11:30:40 +0000
@@ -394,13 +394,21 @@
return report_error(L, "Unknown Building: '%s'", name);
Building * b = 0;
- if (force)
- b = &get(L, get_egbase(L)).force_building
- (c->coords(), i, constructionsite);
- else
+ if (force) {
+ if (constructionsite) {
+ b = &get(L, get_egbase(L)).force_csite
+ (c->coords(), i);
+ } else {
+ Building_Descr::FormerBuildings former_buildings;
+ const Building_Descr * descr = get(L, get_egbase(L)).tribe().get_building_descr(i);
+ former_buildings.push_back(descr);
+ b = &get(L, get_egbase(L)).force_building
+ (c->coords(), former_buildings);
+ }
+ } else {
b = get(L, get_egbase(L)).build
(c->coords(), i, constructionsite);
-
+ }
if (not b)
return report_error(L, "Couldn't place building!");
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
