Merge lp:~widelands-dev/widelands/bug-1455732 into lp:widelands

[GunChleoc]

This fixes a bug in the Tribal Encyclopedia where Dismantlesites weren't persisted.

We still have a bug with the Constructionsite that I can't figure out.

[Hans Joachim Desserud (hjd)]

Hi, I did some quick testing of this patch. I found that if any mine is selected, I get a similar crash when attempting to save. I guess that needs a similar fix.

I saw some crashes when selecting dismantling site and then something else, but those might have been caused by the other building happening to be a mine. Hard to tell, but I can check again once that's fixed.

I only skimmed the code changes, but thumbs up for replacing the if statements with a switch, :)

review: Needs Fixing

[GunChleoc (gunchleoc)]

Good catch, the culprit is actually the mine, not the construction site.

[GunChleoc (gunchleoc)]

Found the other bug - a global variable in Lua. The bug has been there for a while, ever since we added the automated building help.

review: Needs Resubmitting

[Hans Joachim Desserud (hjd)]

I am no longer able to trigger a crash when selecting things in the building list and saving. :)

I have not reviewed the code.

[TiborB (tiborb95)]

I have not tested it but the code looks good to me :)

review: Approve

[GunChleoc (gunchleoc)]

I think we're good to go then. Will merge.

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2015-05-10 10:59:28 +0000
+++ src/scripting/lua_map.cc	2015-05-21 06:58:26 +0000
@@ -550,6 +550,8 @@
 		switch (descr->type()) {
 			case MapObjectType::CONSTRUCTIONSITE:
 				return CAST_TO_LUA(ConstructionSiteDescr, LuaConstructionSiteDescription);
+			case MapObjectType::DISMANTLESITE:
+				return CAST_TO_LUA(DismantleSiteDescr, LuaDismantleSiteDescription);
 			case MapObjectType::PRODUCTIONSITE:
 				return CAST_TO_LUA(ProductionSiteDescr, LuaProductionSiteDescription);
 			case MapObjectType::MILITARYSITE:
@@ -1357,6 +1359,26 @@
 
 
 /* RST
+DismantleSiteDescription
+---------------------------
+
+.. class:: DismantleSiteDescription
+
+	 A static description of a tribe's dismantlesite, so it can be used in help files
+	 without having to access an actual building on the map.
+	 See also class BuildingDescription and class MapObjectDescription for more properties.
+*/
+const char LuaDismantleSiteDescription::className[] = "DismantleSiteDescription";
+const MethodType<LuaDismantleSiteDescription> LuaDismantleSiteDescription::Methods[] = {
+	{nullptr, nullptr},
+};
+const PropertyType<LuaDismantleSiteDescription> LuaDismantleSiteDescription::Properties[] = {
+	{nullptr, nullptr, nullptr},
+};
+
+
+
+/* RST
 ProductionSiteDescription
 -------------------------
 
@@ -2085,31 +2107,30 @@
 /* RST
     .. attribute:: descr
 
-        (RO) The description object for this immovable, e.g. BuildingDescription.
+		  (RO) The description object for this immovable, e.g. BuildingDescription.
 */
 int LuaMapObject::get_descr(lua_State * L) {
 	const MapObjectDescr* desc = &get(L, get_egbase(L))->descr();
 	assert(desc != nullptr);
 
-	if (is_a(MilitarySiteDescr, desc)) {
-		return CAST_TO_LUA(MilitarySiteDescr, LuaMilitarySiteDescription);
-	}
-	else if (is_a(TrainingSiteDescr, desc)) {
-		return CAST_TO_LUA(TrainingSiteDescr, LuaTrainingSiteDescription);
-	}
-	else if (is_a(ProductionSiteDescr, desc)) {
-		return CAST_TO_LUA(ProductionSiteDescr, LuaProductionSiteDescription);
-	}
-	else if (is_a(WarehouseDescr, desc)) {
-		return CAST_TO_LUA(WarehouseDescr, LuaWarehouseDescription);
-	}
-	else if (is_a(ConstructionSiteDescr, desc)) {
-		return CAST_TO_LUA(ConstructionSiteDescr, LuaConstructionSiteDescription);
-	}
-	else if (is_a(BuildingDescr, desc)) {
-		return CAST_TO_LUA(BuildingDescr, LuaBuildingDescription);
-	}
-	return CAST_TO_LUA(MapObjectDescr, LuaMapObjectDescription);
+	switch (desc->type()) {
+		case (MapObjectType::BUILDING):
+			return CAST_TO_LUA(BuildingDescr, LuaBuildingDescription);
+		case (MapObjectType::CONSTRUCTIONSITE):
+			return CAST_TO_LUA(ConstructionSiteDescr, LuaConstructionSiteDescription);
+		case (MapObjectType::DISMANTLESITE):
+			return CAST_TO_LUA(DismantleSiteDescr, LuaDismantleSiteDescription);
+		case (MapObjectType::PRODUCTIONSITE):
+			return CAST_TO_LUA(ProductionSiteDescr, LuaProductionSiteDescription);
+		case (MapObjectType::MILITARYSITE):
+			return CAST_TO_LUA(MilitarySiteDescr, LuaMilitarySiteDescription);
+		case (MapObjectType::TRAININGSITE):
+			return CAST_TO_LUA(TrainingSiteDescr, LuaTrainingSiteDescription);
+		case (MapObjectType::WAREHOUSE):
+			return CAST_TO_LUA(WarehouseDescr, LuaWarehouseDescription);
+		default:
+			return CAST_TO_LUA(MapObjectDescr, LuaMapObjectDescription);
+	}
 }
 
 #undef CAST_TO_LUA
@@ -4452,6 +4473,11 @@
 	add_parent<LuaConstructionSiteDescription, LuaMapObjectDescription>(L);
 	lua_pop(L, 1); // Pop the meta table
 
+	register_class<LuaDismantleSiteDescription>(L, "map", true);
+	add_parent<LuaDismantleSiteDescription, LuaBuildingDescription>(L);
+	add_parent<LuaDismantleSiteDescription, LuaMapObjectDescription>(L);
+	lua_pop(L, 1); // Pop the meta table
+
 	register_class<LuaProductionSiteDescription>(L, "map", true);
 	add_parent<LuaProductionSiteDescription, LuaBuildingDescription>(L);
 	add_parent<LuaProductionSiteDescription, LuaMapObjectDescription>(L);

=== modified file 'src/scripting/lua_map.h'
--- src/scripting/lua_map.h	2015-05-10 10:59:28 +0000
+++ src/scripting/lua_map.h	2015-05-21 06:58:26 +0000
@@ -26,6 +26,7 @@
 #include "economy/portdock.h"
 #include "economy/road.h"
 #include "logic/constructionsite.h"
+#include "logic/dismantlesite.h"
 #include "logic/game.h"
 #include "logic/militarysite.h"
 #include "logic/productionsite.h"
@@ -204,22 +205,28 @@
 	LuaConstructionSiteDescription(lua_State* L) : LuaBuildingDescription(L) {
 	}
 
-	/*
-	 * Properties
-	 */
-
-	/*
-	 * Lua methods
-	 */
-
-	/*
-	 * C methods
-	 */
-
 private:
 	CASTED_GET_DESCRIPTION(ConstructionSiteDescr)
 };
 
+class LuaDismantleSiteDescription : public LuaBuildingDescription {
+public:
+	LUNA_CLASS_HEAD(LuaDismantleSiteDescription);
+
+	virtual ~LuaDismantleSiteDescription() {}
+
+	LuaDismantleSiteDescription() {}
+	LuaDismantleSiteDescription(const Widelands::DismantleSiteDescr* const dismantlesitedescr)
+		: LuaBuildingDescription(dismantlesitedescr) {
+	}
+	LuaDismantleSiteDescription(lua_State* L) : LuaBuildingDescription(L) {
+	}
+
+private:
+	CASTED_GET_DESCRIPTION(DismantleSiteDescr)
+};
+
+
 
 class LuaProductionSiteDescription : public LuaBuildingDescription {
 public:

=== modified file 'tribes/scripting/format_help.lua'
--- tribes/scripting/format_help.lua	2014-10-19 12:02:45 +0000
+++ tribes/scripting/format_help.lua	2015-05-21 06:58:26 +0000
@@ -120,7 +120,7 @@
 	if not text then
 		text = ""
 	end
-	string = "image=tribes/" .. tribename .. "/" .. resource  .. "/resi_00.png"
+	local string = "image=tribes/" .. tribename .. "/" .. resource  .. "/resi_00.png"
 	for k,v in ipairs({table.unpack(items)}) do
 		string = string .. ";pics/arrow-right.png;" ..  v.icon_name
 	end