cghislai has proposed merging lp:~widelands-dev/widelands/formerbuildings_index
into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1205149 in widelands: "Crash on Ubuntu 12.04 when clicking to open a
building window"
https://bugs.launchpad.net/widelands/+bug/1205149
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/formerbuildings_index/+merge/179570
Previously former building information was stored as Building_Descr pointers.
Here it is stored as building_Index values.
It prevents the crash to happen on Ubuntu 12.04.
--
https://code.launchpad.net/~widelands-dev/widelands/formerbuildings_index/+merge/179570
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/formerbuildings_index into lp:widelands.
=== modified file 'src/logic/building.cc'
--- src/logic/building.cc 2013-08-08 19:55:12 +0000
+++ src/logic/building.cc 2013-08-10 12:30:44 +0000
@@ -193,8 +193,8 @@
Building & b = construct ? create_constructionsite() : create_object();
b.m_position = pos;
b.set_owner(&owner);
- BOOST_FOREACH(const Building_Descr * descr, former_buildings) {
- b.m_old_buildings.push_back(descr);
+ BOOST_FOREACH(Building_Index idx, former_buildings) {
+ b.m_old_buildings.push_back(idx);
}
if (loading) {
b.Building::init(egbase);
=== modified file 'src/logic/building.h'
--- src/logic/building.h 2013-08-08 19:55:12 +0000
+++ src/logic/building.h 2013-08-10 12:30:44 +0000
@@ -60,7 +60,7 @@
*/
struct Building_Descr : public Map_Object_Descr {
typedef std::set<Building_Index> Enhancements;
- typedef std::vector<const Building_Descr*> FormerBuildings;
+ typedef std::vector<Building_Index> FormerBuildings;
Building_Descr
(char const * _name, char const * _descname,
@@ -176,7 +176,7 @@
PCap_Enhancable = 1 << 2, // can be enhanced to something
};
- typedef std::vector<const Building_Descr*> FormerBuildings;
+ typedef std::vector<Building_Index> FormerBuildings;
public:
Building(const Building_Descr &);
@@ -240,9 +240,9 @@
/**
* The former buildings vector keeps track of all former buildings
* that have been enhanced up to the current one. The current building
- * descr will be in the last position. For construction sites, it is
- * empty except if a former building is being enhanced. For a dismantle
- * site, the last item will be the one being dismantled.
+ * index will be in the last position. For construction sites, it is
+ * empty exceptenhancements. For a dismantle site, the last item will
+ * be the one being dismantled.
*/
const FormerBuildings get_former_buildings() {
return m_old_buildings;
@@ -311,7 +311,7 @@
// Signals connected for the option window
std::vector<boost::signals2::connection> options_window_connections;
- // The former buildings descrs, with the current one in last position.
+ // The former buildings names, with the current one in last position.
FormerBuildings m_old_buildings;
};
=== modified file 'src/logic/constructionsite.cc'
--- src/logic/constructionsite.cc 2013-07-26 20:19:36 +0000
+++ src/logic/constructionsite.cc 2013-08-10 12:30:44 +0000
@@ -132,7 +132,9 @@
const std::map<Ware_Index, uint8_t> * buildcost;
if (!m_old_buildings.empty()) {
// Enhancement
- m_info.was = m_old_buildings.back();
+ Building_Index was_index = m_old_buildings.back();
+ const Building_Descr* was_descr = tribe().get_building_descr(was_index);
+ m_info.was = was_descr;
buildcost = &m_building->enhancement_cost();
} else {
buildcost = &m_building->buildcost();
@@ -169,7 +171,8 @@
if (m_work_steps <= m_work_completed) {
// Put the real building in place
- m_old_buildings.push_back(m_building);
+ Building_Index becomes_idx = tribe().building_index(m_building->name());
+ m_old_buildings.push_back(becomes_idx);
Building & b =
m_building->create(egbase, owner(), m_position, false, false, m_old_buildings);
if (Worker * const builder = m_builder.get(egbase)) {
@@ -387,7 +390,8 @@
// draw the prev pic from top to where next image will be drawing
dst.drawanimrect(pos, anim, tanim - FRAME_LENGTH, get_owner(), Rect(Point(0, 0), w, h - lines));
else if (!m_old_buildings.empty()) {
- const Building_Descr* prev_building = m_old_buildings.back();
+ Building_Index prev_idx = m_old_buildings.back();
+ const Building_Descr* prev_building = tribe().get_building_descr(prev_idx);
// Is the first picture but there was another building here before,
// get its most fitting picture and draw it instead.
uint32_t a;
=== modified file 'src/logic/dismantlesite.cc'
--- src/logic/dismantlesite.cc 2013-07-28 09:02:37 +0000
+++ src/logic/dismantlesite.cc 2013-08-10 12:30:44 +0000
@@ -71,10 +71,11 @@
Partially_Finished_Building(gdescr)
{
assert(!former_buildings.empty());
- BOOST_FOREACH(const Building_Descr* old_descr, former_buildings) {
- m_old_buildings.push_back(old_descr);
+ BOOST_FOREACH(Building_Index former_idx, former_buildings) {
+ m_old_buildings.push_back(former_idx);
}
- set_building(*m_old_buildings.back());
+ const Building_Descr* cur_descr = owner().tribe().get_building_descr(m_old_buildings.back());
+ set_building(*cur_descr);
m_position = c;
set_owner(&plr);
@@ -133,9 +134,10 @@
(Building* building,
std::map<Ware_Index, uint8_t> & res)
{
- BOOST_FOREACH(const Building_Descr* former_descr, building->get_former_buildings()) {
+ BOOST_FOREACH(Building_Index former_idx, building->get_former_buildings()) {
const std::map<Ware_Index, uint8_t> * return_wares;
- if (former_descr != building->get_former_buildings().front()) {
+ const Building_Descr* former_descr = building->tribe().get_building_descr(former_idx);
+ if (former_idx != building->get_former_buildings().front()) {
return_wares = & former_descr->returned_wares_enhanced();
} else {
return_wares = & former_descr->returned_wares();
=== modified file 'src/logic/militarysite.cc'
--- src/logic/militarysite.cc 2013-08-08 19:55:12 +0000
+++ src/logic/militarysite.cc 2013-08-10 12:30:44 +0000
@@ -867,8 +867,9 @@
// 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) {
+ Building::FormerBuildings former_buildings;
+ BOOST_FOREACH(Building_Index former_idx, m_old_buildings) {
+ const Building_Descr * old_descr = tribe().get_building_descr(former_idx);
std::string bldname = old_descr->name();
// Has this building already a suffix? == conquered building?
std::string::size_type const dot = bldname.rfind('.');
@@ -879,8 +880,7 @@
} 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);
+ former_buildings.push_back(bldi);
}
// Now we destroy the old building before we place the new one.
=== modified file 'src/logic/player.cc'
--- src/logic/player.cc 2013-08-08 20:02:13 +0000
+++ src/logic/player.cc 2013-08-10 12:30:44 +0000
@@ -110,19 +110,18 @@
*/
void find_former_buildings
(const Widelands::Tribe_Descr & tribe_descr, const Widelands::Building_Index bi,
- Widelands::Building_Descr::FormerBuildings* former_buildings)
+ Widelands::Building::FormerBuildings* former_buildings)
{
assert(former_buildings && former_buildings->empty());
- const Widelands::Building_Descr * first_descr = tribe_descr.get_building_descr(bi);
- former_buildings->push_back(first_descr);
+ former_buildings->push_back(bi);
bool done = false;
while (not done) {
- const Widelands::Building_Descr * oldest = former_buildings->front();
+ Widelands::Building_Index oldest_idx = former_buildings->front();
+ const Widelands::Building_Descr * oldest = tribe_descr.get_building_descr(oldest_idx);
if (!oldest->is_enhanced()) {
done = true;
break;
}
- const Widelands::Building_Index & oldest_idx = tribe_descr.building_index(oldest->name());
for
(Widelands::Building_Index i = Widelands::Building_Index::First();
i < tribe_descr.get_nrbuildings();
@@ -130,7 +129,7 @@
{
const Widelands::Building_Descr* ob = tribe_descr.get_building_descr(i);
if (ob->enhancements().count(oldest_idx)) {
- former_buildings->insert(former_buildings->begin(), ob);
+ former_buildings->insert(former_buildings->begin(), i);
break;
}
}
@@ -506,7 +505,8 @@
const Building_Descr::FormerBuildings & former_buildings)
{
Map & map = egbase().map();
- const Building_Descr* descr = former_buildings.back();
+ Building_Index idx = former_buildings.back();
+ const Building_Descr* descr = tribe().get_building_descr(idx);
terraform_for_building(egbase(), player_number(), location, descr);
FCoords flag_loc;
map.get_brn(map.get_fcoords(location), &flag_loc);
@@ -522,7 +522,8 @@
const Building_Descr::FormerBuildings & former_buildings)
{
Map & map = egbase().map();
- const Building_Descr * descr = tribe().get_building_descr(b_idx);
+ Building_Index idx = former_buildings.back();
+ const Building_Descr* descr = tribe().get_building_descr(idx);
terraform_for_building(egbase(), player_number(), location, descr);
FCoords flag_loc;
map.get_brn(map.get_fcoords(location), &flag_loc);
=== modified file 'src/logic/player.h'
--- src/logic/player.h 2013-08-01 10:51:41 +0000
+++ src/logic/player.h 2013-08-10 12:30:44 +0000
@@ -31,6 +31,7 @@
#include "logic/warehouse.h"
#include "logic/widelands.h"
+class Node;
namespace Widelands {
class Economy;
@@ -398,15 +399,14 @@
throw ();
/// Call see_node for each node in the area.
- void see_area(const Area<FCoords>& area)
- throw ()
- {
+ void see_area(const Area<FCoords>& area) throw () {
const Time gametime = egbase().get_gametime();
const Map & map = egbase().map();
const Widelands::Field & first_map_field = map[0];
MapRegion<Area<FCoords> > mr(map, area);
- do see_node(map, first_map_field, mr.location(), gametime);
- while (mr.advance(map));
+ do {
+ see_node(map, first_map_field, mr.location(), gametime);
+ } while (mr.advance(map));
m_view_changed = true;
}
@@ -458,12 +458,12 @@
Road * build_road(const Path &); /// Build a road if it is allowed.
Building & force_building
(const Coords,
- const Building_Descr::FormerBuildings &);
+ const Building::FormerBuildings &);
Building & force_csite
(const Coords,
Building_Index,
- const Building_Descr::FormerBuildings & = Building_Descr::FormerBuildings());
- Building * build(Coords, Building_Index, bool, Building_Descr::FormerBuildings &);
+ const Building::FormerBuildings & = Building::FormerBuildings());
+ Building * build(Coords, Building_Index, bool, Building::FormerBuildings &);
void bulldoze(PlayerImmovable &, bool recurse = false);
void flagaction(Flag &);
void start_stop_building(PlayerImmovable &);
@@ -649,7 +649,7 @@
void find_former_buildings
(const Tribe_Descr & tribe_descr, const Building_Index bi,
- Building_Descr::FormerBuildings* former_buildings);
+ Building::FormerBuildings* former_buildings);
}
=== modified file 'src/logic/playercommand.cc'
--- src/logic/playercommand.cc 2013-08-07 12:32:36 +0000
+++ src/logic/playercommand.cc 2013-08-10 12:30:44 +0000
@@ -226,7 +226,7 @@
void Cmd_Build::execute (Game & game)
{
// Empty former vector since its a new csite.
- Building_Descr::FormerBuildings former_buildings;
+ Building::FormerBuildings former_buildings;
game.player(sender()).build(coords, bi, true, former_buildings);
}
=== modified file 'src/map_io/widelands_map_building_data_packet.cc'
--- src/map_io/widelands_map_building_data_packet.cc 2013-07-31 17:00:28 +0000
+++ src/map_io/widelands_map_building_data_packet.cc 2013-08-10 12:30:44 +0000
@@ -87,8 +87,7 @@
if (special_type == 1) { // Constructionsite
building = &egbase.warp_constructionsite(c, p, index, true);
} else if (special_type == 2) {// DismantleSite
- const Building_Descr* former_desc = tribe.get_building_descr(index);
- Building::FormerBuildings formers = {former_desc};
+ Building::FormerBuildings formers = {index};
building = &egbase.warp_dismantlesite(c, p, true, formers);
} else {
building = &egbase.warp_building(c, p, index);
=== modified file 'src/map_io/widelands_map_buildingdata_data_packet.cc'
--- src/map_io/widelands_map_buildingdata_data_packet.cc 2013-08-01 03:23:11 +0000
+++ src/map_io/widelands_map_buildingdata_data_packet.cc 2013-08-10 12:30:44 +0000
@@ -153,13 +153,15 @@
// will be built after other data are loaded, see below.
// read_formerbuildings_v2()
while (fr.Unsigned8()) {
- const Building_Descr* former_descr =
- building.descr().tribe().get_building_descr
- (building.descr().tribe().safe_building_index(fr.CString()));
- building.m_old_buildings.push_back(former_descr);
+ Building_Index oldidx = building.descr().tribe().safe_building_index(fr.CString());
+ building.m_old_buildings.push_back(oldidx);
}
// Only construction sites may have an empty list
- assert(!building.m_old_buildings.empty() || is_a(ConstructionSite, &building));
+ if (building.m_old_buildings.empty() && !is_a(ConstructionSite, &building)) {
+ throw game_data_error
+ ("Failed to read %s %u: No former buildings informations.\n"
+ "Your savegame is corrupted", building.descr().descname().c_str(), building.serial());
+ }
}
if (fr.Unsigned8()) {
if (upcast(ProductionSite, productionsite, &building))
@@ -251,12 +253,14 @@
void Map_Buildingdata_Data_Packet::read_formerbuildings_v2
(Building& b, FileRead&, Game&, Map_Map_Object_Loader&)
{
+ const Tribe_Descr & t = b.descr().tribe();
+ Building_Index b_idx = t.building_index(b.descr().name());
if (is_a(ProductionSite, &b)) {
assert(b.m_old_buildings.empty());
- b.m_old_buildings.push_back(&b.descr());
+ b.m_old_buildings.push_back(b_idx);
} else if (is_a(Warehouse, &b)) {
assert(b.m_old_buildings.empty());
- b.m_old_buildings.push_back(&b.descr());
+ b.m_old_buildings.push_back(b_idx);
} else if (upcast(DismantleSite, dsite, &b)) {
// Former buildings filled with the current one
// upon building init.
@@ -269,17 +273,16 @@
}
// iterate through all buildings to find first predecessor
- const Tribe_Descr & t = b.descr().tribe();
for (;;) {
- const Building_Descr * oldest = b.m_old_buildings.front();
+ Building_Index former_idx = b.m_old_buildings.front();
+ const Building_Descr * oldest = t.get_building_descr(former_idx);
if (!oldest->is_enhanced()) {
break;
}
- const Building_Index & oldest_idx = t.building_index(oldest->name());
for (Building_Index i = Building_Index::First(); i < t.get_nrbuildings(); ++i) {
Building_Descr const * ob = t.get_building_descr(i);
- if (ob->enhancements().count(oldest_idx)) {
- b.m_old_buildings.insert(b.m_old_buildings.begin(), ob);
+ if (ob->enhancements().count(former_idx)) {
+ b.m_old_buildings.insert(b.m_old_buildings.begin(), i);
break;
}
}
@@ -371,10 +374,8 @@
if (packet_version <= 2) {
if (fr.Unsigned8()) {
- const Building_Descr* former_descr =
- tribe.get_building_descr
- (tribe.safe_building_index(fr.CString()));
- constructionsite.m_old_buildings.push_back(former_descr);
+ Building_Index idx = tribe.safe_building_index(fr.CString());
+ constructionsite.m_old_buildings.push_back(idx);
}
}
@@ -397,10 +398,8 @@
constructionsite.m_building =
tribe.get_building_descr(tribe.safe_building_index(fr.CString()));
if (fr.Unsigned8()) {
- const Building_Descr * former_descr =
- tribe.get_building_descr
- (tribe.safe_building_index(fr.CString()));
- constructionsite.m_old_buildings.push_back(former_descr);
+ Building_Index bidx = tribe.safe_building_index(fr.CString());
+ constructionsite.m_old_buildings.push_back(bidx);
}
delete constructionsite.m_builder_request;
@@ -1318,9 +1317,11 @@
fw.Unsigned32(0);
}
{
- BOOST_FOREACH(const Building_Descr* former_descr, building->m_old_buildings) {
+ const Tribe_Descr& td = building->descr().tribe();
+ BOOST_FOREACH(Building_Index b_idx, building->m_old_buildings) {
+ const Building_Descr* b_descr = td.get_building_descr(b_idx);
fw.Unsigned8(1);
- fw.String(former_descr->name());
+ fw.String(b_descr->name());
}
fw.Unsigned8(0);
}
_______________________________________________
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
