widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1544864 into lp:widelands.

Commit message:
Removed LuaBaseImmovable::get_size. The corresponding functions in LuaBuildingDescription and LuaImmovableDescription now return strings instead of ints for consistency and easier reading.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1544864 in widelands: "Get rid of LuaBaseImmovable::get_size"
  https://bugs.launchpad.net/widelands/+bug/1544864

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1544864/+merge/285994
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1544864 into lp:widelands.

=== modified file 'data/tribes/scripting/help/building_help.lua'
--- data/tribes/scripting/help/building_help.lua	2016-01-28 05:24:34 +0000
+++ data/tribes/scripting/help/building_help.lua	2016-02-14 16:39:57 +0000
@@ -449,11 +449,11 @@
    elseif (building_description.is_port) then
       result = result .. text_line(_"Space required:",_"Port plot","images/wui/overlays/port.png")
    else
-      if (building_description.size == 1) then
+      if (building_description.size == "small") then
          result = result .. text_line(_"Space required:",_"Small plot","images/wui/overlays/small.png")
-      elseif (building_description.size == 2) then
+      elseif (building_description.size == "medium") then
          result = result .. text_line(_"Space required:",_"Medium plot","images/wui/overlays/medium.png")
-      elseif (building_description.size == 3) then
+      elseif (building_description.size == "big") then
          result = result .. text_line(_"Space required:",_"Big plot","images/wui/overlays/big.png")
       else
          result = result .. p(_"Space required:" .. _"Unknown")

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2016-02-14 09:59:59 +0000
+++ src/scripting/lua_map.cc	2016-02-14 16:39:57 +0000
@@ -1550,20 +1550,32 @@
 /* RST
 	.. attribute:: size
 
-			(RO) the size of the immovable as an int.
+		(RO) The size of this immovable. Can be either of
+
+		* :const:`none` -- Example: mushrooms. Immovables will be destroyed when
+			something else is built on this field.
+		* :const:`small` -- Example: trees or flags
+		* :const:`medium` -- Example: Medium sized buildings
+		* :const:`big` -- Example: Big sized buildings or rocks
 */
 int LuaImmovableDescription::get_size(lua_State * L) {
-	// TODO(GunChleoc): see this todo, that is also mentioned below for
-	// buildings. I think we can do that now, every description is wrapped I
-	// think. Essentially that means every instance of something on the map
-	// (like a building) get's a .description that has the static data for the
-	// building/immovable. maybe in a followup branch, but definitvely before b19, since that is backwards
-	// incompatible.
-	// TODO(SirVer): size should be similar to
-	// https://wl.widelands.org/docs/wl/autogen_wl_map/#wl.map.BaseImmovable.size.
-	// In fact, as soon as all descriptions are wrapped (also for other
-	// immovables besides buildings) we should get rid of BaseImmovable.size.
-	lua_pushinteger(L, get()->get_size());
+	switch (get()->get_size()) {
+	case BaseImmovable::NONE:
+		lua_pushstring(L, "none");
+		break;
+	case BaseImmovable::SMALL:
+		lua_pushstring(L, "small");
+		break;
+	case BaseImmovable::MEDIUM:
+		lua_pushstring(L, "medium");
+		break;
+	case BaseImmovable::BIG:
+		lua_pushstring(L, "big");
+		break;
+	default:
+		report_error(L, "Unknown size %i in LuaImmovableDescription::get_size: %s",
+						 get()->get_size(), get()->name().c_str());
+	}
 	return 1;
 }
 
@@ -1642,13 +1654,9 @@
 	PROP_RO(LuaBuildingDescription, enhancement),
 	PROP_RO(LuaBuildingDescription, is_mine),
 	PROP_RO(LuaBuildingDescription, is_port),
+	PROP_RO(LuaBuildingDescription, size),
 	PROP_RO(LuaBuildingDescription, returned_wares),
 	PROP_RO(LuaBuildingDescription, returned_wares_enhanced),
-	// TODO(SirVer): size should be similar to
-	// https://wl.widelands.org/docs/wl/autogen_wl_map/#wl.map.BaseImmovable.size.
-	// In fact, as soon as all descriptions are wrapped (also for other
-	// immovables besides buildings) we should get rid of BaseImmovable.size.
-	PROP_RO(LuaBuildingDescription, size),
 	PROP_RO(LuaBuildingDescription, vision_range),
 	PROP_RO(LuaBuildingDescription, workarea_radius),
 	{nullptr, nullptr, nullptr},
@@ -1801,6 +1809,33 @@
 }
 
 /* RST
+	.. attribute:: size
+
+		(RO) The size of this building. Can be either of
+
+		* :const:`small` -- Small sized buildings
+		* :const:`medium` -- Medium sized buildings
+		* :const:`big` -- Big sized buildings
+*/
+int LuaBuildingDescription::get_size(lua_State * L) {
+	switch (get()->get_size()) {
+	case BaseImmovable::SMALL:
+		lua_pushstring(L, "small");
+		break;
+	case BaseImmovable::MEDIUM:
+		lua_pushstring(L, "medium");
+		break;
+	case BaseImmovable::BIG:
+		lua_pushstring(L, "big");
+		break;
+	default:
+		report_error(L, "Unknown size %i in LuaBuildingDescription::get_size: %s",
+						 get()->get_size(), get()->name().c_str());
+	}
+	return 1;
+}
+
+/* RST
 	.. attribute:: returned_wares
 
 			(RO) a list of wares returned upon dismantling.
@@ -1821,17 +1856,6 @@
 
 
 /* RST
-	.. attribute:: size
-
-			(RO) the size of the building: 1 = small, 2 = medium, 3 = big.
-*/
-int LuaBuildingDescription::get_size(lua_State * L) {
-	lua_pushinteger(L, get()->get_size());
-	return 1;
-}
-
-
-/* RST
 	.. attribute:: vision range
 
 			(RO) the vision_range of the building as an int.
@@ -3095,6 +3119,10 @@
 			return CAST_TO_LUA(TrainingSiteDescr, LuaTrainingSiteDescription);
 		case (MapObjectType::WAREHOUSE):
 			return CAST_TO_LUA(WarehouseDescr, LuaWarehouseDescription);
+		case (MapObjectType::IMMOVABLE):
+			return CAST_TO_LUA(ImmovableDescr, LuaImmovableDescription);
+		case (MapObjectType::WORKER):
+			return CAST_TO_LUA(WorkerDescr, LuaWorkerDescription);
 		default:
 			return CAST_TO_LUA(MapObjectDescr, LuaMapObjectDescription);
 	}
@@ -3213,7 +3241,6 @@
 	{nullptr, nullptr},
 };
 const PropertyType<LuaBaseImmovable> LuaBaseImmovable::Properties[] = {
-	PROP_RO(LuaBaseImmovable, size),
 	PROP_RO(LuaBaseImmovable, fields),
 	{nullptr, nullptr, nullptr},
 };
@@ -3223,30 +3250,6 @@
  PROPERTIES
  ==========================================================
  */
-/* RST
-	.. attribute:: size
-
-		(RO) The size of this immovable. Can be either of
-
-		* :const:`none` -- Example: mushrooms. Immovables will be destroyed when
-			something else is build on this field.
-		* :const:`small` -- Example: trees or flags
-		* :const:`medium` -- Example: Medium sized buildings
-		* :const:`big` -- Example: Big sized buildings or rocks
-*/
-int LuaBaseImmovable::get_size(lua_State * L) {
-	BaseImmovable * o = get(L, get_egbase(L));
-
-	switch (o->get_size()) {
-		case BaseImmovable::NONE: lua_pushstring(L, "none"); break;
-		case BaseImmovable::SMALL: lua_pushstring(L, "small"); break;
-		case BaseImmovable::MEDIUM: lua_pushstring(L, "medium"); break;
-		case BaseImmovable::BIG: lua_pushstring(L, "big"); break;
-		default:
-			report_error(L, "Unknown size in LuaBaseImmovable::get_size: %i", o->get_size());
-	}
-	return 1;
-}
 
 /* RST
 	.. attribute:: fields

=== modified file 'src/scripting/lua_map.h'
--- src/scripting/lua_map.h	2016-02-13 13:03:55 +0000
+++ src/scripting/lua_map.h	2016-02-14 16:39:57 +0000
@@ -277,10 +277,10 @@
 	int get_enhancement(lua_State *);
 	int get_is_mine(lua_State *);
 	int get_is_port(lua_State *);
+	int get_size(lua_State *);
 	int get_isproductionsite(lua_State *);
 	int get_returned_wares(lua_State *);
 	int get_returned_wares_enhanced(lua_State *);
-	int get_size(lua_State *);
 	int get_vision_range(lua_State *);
 	int get_workarea_radius(lua_State *);
 
@@ -726,7 +726,6 @@
 	/*
 	 * Properties
 	 */
-	int get_size(lua_State * L);
 	int get_fields(lua_State * L);
 
 	/*

=== modified file 'test/maps/lua_testsuite.wmf/scripting/baseimmovables.lua'
--- test/maps/lua_testsuite.wmf/scripting/baseimmovables.lua	2015-10-31 15:04:53 +0000
+++ test/maps/lua_testsuite.wmf/scripting/baseimmovables.lua	2016-02-14 16:39:57 +0000
@@ -116,19 +116,19 @@
 end
 
 function immovable_property_tests:test_size_none()
-   assert_equal("none", self.none.size)
+   assert_equal("none", self.none.descr.size)
 end
 function immovable_property_tests:test_size_small()
-   assert_equal("small", self.small.size)
+   assert_equal("small", self.small.descr.size)
 end
 function immovable_property_tests:test_size_medium()
-   assert_equal("medium", self.medium.size)
+   assert_equal("medium", self.medium.descr.size)
 end
 function immovable_property_tests:test_size_big()
-   assert_equal("big", self.big.size)
+   assert_equal("big", self.big.descr.size)
 end
 function immovable_property_tests:test_size_fortress()
-   assert_equal("big", self.big_building.size)
+   assert_equal("big", self.big_building.descr.size)
 end
 function immovable_property_tests:test_name_pebble()
    assert_equal("pebble1", self.none.descr.name)

=== modified file 'test/maps/lua_testsuite.wmf/scripting/constructionsite.lua'
--- test/maps/lua_testsuite.wmf/scripting/constructionsite.lua	2015-10-31 12:11:44 +0000
+++ test/maps/lua_testsuite.wmf/scripting/constructionsite.lua	2016-02-14 16:39:57 +0000
@@ -36,8 +36,10 @@
 end
 
 function constructionsite_tests:test_size()
-   assert_equal("small", self.l.size)
-   assert_equal("big", self.f.size)
+
+local building = egbase:get_building_description(self.l.building)
+   assert_equal("small", egbase:get_building_description(self.l.building).size)
+   assert_equal("big", egbase:get_building_description(self.f.building).size)
 end
 
 function constructionsite_tests:test_building()

=== modified file 'test/maps/lua_testsuite.wmf/scripting/immovables_descriptions.lua'
--- test/maps/lua_testsuite.wmf/scripting/immovables_descriptions.lua	2016-02-13 13:03:55 +0000
+++ test/maps/lua_testsuite.wmf/scripting/immovables_descriptions.lua	2016-02-14 16:39:57 +0000
@@ -107,10 +107,10 @@
 end
 
 function test_descr:test_immovable_size()
-   assert_equal(0, egbase:get_immovable_description("bush1").size)
-   assert_equal(1, egbase:get_immovable_description("cornfield_ripe").size)
-   assert_equal(1, egbase:get_immovable_description("alder_summer_sapling").size)
-   assert_equal(1, egbase:get_immovable_description("alder_summer_old").size)
+   assert_equal("none", egbase:get_immovable_description("bush1").size)
+   assert_equal("small", egbase:get_immovable_description("cornfield_ripe").size)
+   assert_equal("small", egbase:get_immovable_description("alder_summer_sapling").size)
+   assert_equal("small", egbase:get_immovable_description("alder_summer_old").size)
 end
 
 function test_descr:test_immovable_has_attribute()
@@ -257,10 +257,11 @@
 end
 
 function test_descr:test_size()
-   assert_equal(1, egbase:get_building_description("barbarians_lumberjacks_hut").size)
-   assert_equal(2, egbase:get_building_description("barbarians_reed_yard").size)
-   assert_equal(3, egbase:get_building_description("barbarians_fortress").size)
-   assert_equal(1, egbase:get_building_description("barbarians_coalmine").size)
+   assert_equal("small", egbase:get_building_description("barbarians_lumberjacks_hut").size)
+   assert_equal("medium", egbase:get_building_description("barbarians_reed_yard").size)
+   assert_equal("big", egbase:get_building_description("barbarians_fortress").size)
+   assert_equal("small", egbase:get_building_description("barbarians_coalmine").size)
+   assert_equal("none", egbase:get_immovable_description("pebble1").size)
 end
 
 function test_descr:test_type()