Merge lp:~widelands-dev/widelands/bug-1675179-lua-economy into lp:widelands

Proposed by GunChleoc
Status: Merged
Merged at revision: 8330
Proposed branch: lp:~widelands-dev/widelands/bug-1675179-lua-economy
Merge into: lp:widelands
Diff against target: 321 lines (+248/-1)
4 files modified
src/scripting/lua_map.cc (+145/-1)
src/scripting/lua_map.h (+50/-0)
test/maps/lua_testsuite.wmf/scripting/geconomy.lua (+52/-0)
test/maps/lua_testsuite.wmf/scripting/init.lua (+1/-0)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1675179-lua-economy
Reviewer Review Type Date Requested Status
Klaus Halfmann compile, review, test Approve
Review via email: mp+321008@code.launchpad.net

Commit message

Added a new object LuaEconomy to LuaMap.

Description of the change

This is for the Empire scenario scripting. The new functions aren't used anywhere in the game yet, so the unit test will have to cover the testing for now.

To post a comment you must log in.
Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Id like to put in some comments, so we can do the review along them.
Please be patient ...

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Where does a lua programme "find" that flag to fetch the economy from?
would it not be easier to get some "default" economy for the headquarter
(like you do in that test :-). Or some warehouses?

Can we have a test for a bare flag? Is there an ecomony for every
disconnected flag?

For the tutotrial this may be enough, but is this stable enoough
for general usage. Can wee add some test where the economies change?

See my inline comments, too

will compile this and let the test run.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Regarding your comments: The economy number is the array index of the economies owned by a player. So, we need both the player number and he economy index to identify it.

When an economy is merged with another economy or its last flag deleted, the vector gets reshuffled. I don't expect it to be a problem with the persistence code though, because the game is paused while its being saved, so economies can't disappear on us then.

Outside of saveloading, we reference the Economy object itself and not the economy & player numbers, so the worst thing that it can do to us is to become nil. Becoming nil can happen to any map object though, so this is something that a scenario Lua script will need to test for when necessary.

> Where does a lua programme "find" that flag to fetch the economy from?

It doesn't. Whoever writes a scenario will need to tell it which flag to use.

> would it not be easier to get some "default" economy for the headquarter
(like you do in that test :-). Or some warehouses?

No, because a scenario author might need to be able to access all economies, not just the "default" one. Also, if the headquarters gets conquered and the player still has 2 other economies with warehouses in them, which one will become the "default" economy?

> Can we have a test for a bare flag? Is there an ecomony for every
disconnected flag?

A disconnected flag has an economy that consists of that flag only. Unless you connect it to a warehouse, it will be an economy without a warehouse, but it's still an economy.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Mhh, ./regression_test.py does not execute geconomy.lua? is there something missing?

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 2064. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/215195452.
Appveyor build 1899. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1675179_lua_economy-1899.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Why do you think that?

It is definitely executed when I run

    ./widelands --scenario=test/maps/lua_testsuite.wmf --datadir_for_testing=.

because it blew up in my face quite a few times until I fixed everything up. And it's registered in the same init.lua that ./regression_test.py will run.

From Travis:

    test/maps/lua_testsuite.wmf/scripting/test_lua_in_game.lua ...
      Running Widelands ... done.
    ok

If you want to make sure that it's run, add a nonsense assert to make it fail. I just did that and it definitely fails then, so it is run.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

OK, checke that testsuite and foud how it is used.
updated again and run the tests agin, all fine.

Sorry fo taking so long.

@bunnybot merge

review: Approve (compile, review, test)
Revision history for this message
GunChleoc (gunchleoc) wrote :

Don't apologize and thanks for reviewing! :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc 2017-01-30 14:40:12 +0000
+++ src/scripting/lua_map.cc 2017-03-29 16:02:45 +0000
@@ -3342,6 +3342,134 @@
3342 */3342 */
33433343
3344/* RST3344/* RST
3345Economy
3346-------
3347.. class:: LuaEconomy
3348
3349 Provides access to an economy. A player can have multiple economies;
3350 you can get an economy from a :class:`Flag`.
3351*/
3352const char LuaEconomy::className[] = "Economy";
3353const MethodType<LuaEconomy> LuaEconomy::Methods[] = {
3354 METHOD(LuaEconomy, ware_target_quantity),
3355 METHOD(LuaEconomy, worker_target_quantity),
3356 METHOD(LuaEconomy, set_ware_target_quantity),
3357 METHOD(LuaEconomy, set_worker_target_quantity),
3358 {nullptr, nullptr},
3359};
3360const PropertyType<LuaEconomy> LuaEconomy::Properties[] = {
3361 {nullptr, nullptr, nullptr},
3362};
3363
3364void LuaEconomy::__persist(lua_State* L) {
3365 const Widelands::Economy* economy = get();
3366 const Widelands::Player& player = economy->owner();
3367 PERS_UINT32("player", player.player_number());
3368 PERS_UINT32("economy", player.get_economy_number(economy));
3369}
3370
3371void LuaEconomy::__unpersist(lua_State* L) {
3372 Widelands::PlayerNumber player_number;
3373 size_t economy_number;
3374 UNPERS_UINT32("player", player_number);
3375 UNPERS_UINT32("economy", economy_number);
3376 const Widelands::Player& player = get_egbase(L).player(player_number);
3377 set_economy_pointer(player.get_economy_by_number(economy_number));
3378}
3379
3380/* RST
3381 .. method:: ware_target_quantity(warename)
3382
3383 Returns the amount of the given ware that should be kept in stock for this economy.
3384
3385 :arg warename: the name of the ware.
3386 :type warename: :class:`string`
3387*/
3388int LuaEconomy::ware_target_quantity(lua_State* L) {
3389 const std::string warename = luaL_checkstring(L, 2);
3390 const Widelands::DescriptionIndex index = get_egbase(L).tribes().ware_index(warename);
3391 if (get_egbase(L).tribes().ware_exists(index)) {
3392 const Widelands::Economy::TargetQuantity& quantity = get()->ware_target_quantity(index);
3393 lua_pushinteger(L, quantity.permanent);
3394 } else {
3395 report_error(L, "There is no ware '%s'.", warename.c_str());
3396 }
3397 return 1;
3398}
3399
3400/* RST
3401 .. method:: worker_target_quantity(workername)
3402
3403 Returns the amount of the given worker that should be kept in stock for this economy.
3404
3405 :arg workername: the name of the worker.
3406 :type workername: :class:`string`
3407*/
3408int LuaEconomy::worker_target_quantity(lua_State* L) {
3409 const std::string workername = luaL_checkstring(L, 2);
3410 const Widelands::DescriptionIndex index = get_egbase(L).tribes().worker_index(workername);
3411 if (get_egbase(L).tribes().worker_exists(index)) {
3412 const Widelands::Economy::TargetQuantity& quantity = get()->worker_target_quantity(index);
3413 lua_pushinteger(L, quantity.permanent);
3414 } else {
3415 report_error(L, "There is no worker '%s'.", workername.c_str());
3416 }
3417 return 1;
3418}
3419
3420/* RST
3421 .. method:: set_ware_target_quantity(warename)
3422
3423 Sets the amount of the given ware type that should be kept in stock for this economy.
3424
3425 :arg warename: the name of the ware type.
3426 :type warename: :class:`string`
3427
3428 :arg amount: the new target amount for the ware. Needs to be >= 0.
3429 :type amount: :class:`integer`
3430*/
3431int LuaEconomy::set_ware_target_quantity(lua_State* L) {
3432 const std::string warename = luaL_checkstring(L, 2);
3433 const Widelands::DescriptionIndex index = get_egbase(L).tribes().ware_index(warename);
3434 if (get_egbase(L).tribes().ware_exists(index)) {
3435 const int quantity = luaL_checkinteger(L, 3);
3436 if (quantity < 0) {
3437 report_error(L, "Target ware quantity needs to be >= 0 but was '%d'.", quantity);
3438 }
3439 get()->set_ware_target_quantity(index, quantity, get_egbase(L).get_gametime());
3440 } else {
3441 report_error(L, "There is no ware '%s'.", warename.c_str());
3442 }
3443 return 1;
3444}
3445
3446/* RST
3447 .. method:: set_worker_target_quantity(workername)
3448
3449 Sets the amount of the given worker type that should be kept in stock for this economy.
3450
3451 :arg workername: the name of the worker type.
3452 :type workername: :class:`string`
3453
3454 :arg amount: the new target amount for the worker. Needs to be >= 0.
3455 :type amount: :class:`integer`
3456*/
3457int LuaEconomy::set_worker_target_quantity(lua_State* L) {
3458 const std::string workername = luaL_checkstring(L, 2);
3459 const Widelands::DescriptionIndex index = get_egbase(L).tribes().worker_index(workername);
3460 if (get_egbase(L).tribes().worker_exists(index)) {
3461 const int quantity = luaL_checkinteger(L, 3);
3462 if (quantity < 0) {
3463 report_error(L, "Target worker quantity needs to be >= 0 but was '%d'.", quantity);
3464 }
3465 get()->set_worker_target_quantity(index, quantity, get_egbase(L).get_gametime());
3466 } else {
3467 report_error(L, "There is no worker '%s'.", workername.c_str());
3468 }
3469 return 1;
3470}
3471
3472/* RST
3345MapObject3473MapObject
3346---------3474---------
33473475
@@ -3682,7 +3810,10 @@
3682 METHOD(LuaFlag, set_wares), METHOD(LuaFlag, get_wares), {nullptr, nullptr},3810 METHOD(LuaFlag, set_wares), METHOD(LuaFlag, get_wares), {nullptr, nullptr},
3683};3811};
3684const PropertyType<LuaFlag> LuaFlag::Properties[] = {3812const PropertyType<LuaFlag> LuaFlag::Properties[] = {
3685 PROP_RO(LuaFlag, roads), PROP_RO(LuaFlag, building), {nullptr, nullptr, nullptr},3813 PROP_RO(LuaFlag, economy),
3814 PROP_RO(LuaFlag, roads),
3815 PROP_RO(LuaFlag, building),
3816 {nullptr, nullptr, nullptr},
3686};3817};
36873818
3688/*3819/*
@@ -3691,6 +3822,18 @@
3691 ==========================================================3822 ==========================================================
3692 */3823 */
3693/* RST3824/* RST
3825 .. attribute:: economy
3826
3827 (RO) Returns the economy that this flag belongs to.
3828
3829 :returns: The :class:`Economy` associated with the flag.
3830*/
3831int LuaFlag::get_economy(lua_State* L) {
3832 const Flag* f = get(L, get_egbase(L));
3833 return to_lua<LuaEconomy>(L, new LuaEconomy(f->get_economy()));
3834}
3835
3836/* RST
3694 .. attribute:: roads3837 .. attribute:: roads
36953838
3696 (RO) Array of roads leading to the flag. Directions3839 (RO) Array of roads leading to the flag. Directions
@@ -6075,6 +6218,7 @@
60756218
6076 register_class<LuaField>(L, "map");6219 register_class<LuaField>(L, "map");
6077 register_class<LuaPlayerSlot>(L, "map");6220 register_class<LuaPlayerSlot>(L, "map");
6221 register_class<LuaEconomy>(L, "map");
6078 register_class<LuaMapObject>(L, "map");6222 register_class<LuaMapObject>(L, "map");
60796223
6080 register_class<LuaBob>(L, "map", true);6224 register_class<LuaBob>(L, "map", true);
60816225
=== modified file 'src/scripting/lua_map.h'
--- src/scripting/lua_map.h 2017-01-25 18:55:59 +0000
+++ src/scripting/lua_map.h 2017-03-29 16:02:45 +0000
@@ -22,6 +22,7 @@
2222
23#include <set>23#include <set>
2424
25#include "economy/economy.h"
25#include "economy/flag.h"26#include "economy/flag.h"
26#include "economy/portdock.h"27#include "economy/portdock.h"
27#include "economy/road.h"28#include "economy/road.h"
@@ -728,6 +729,54 @@
728 const Widelands::TerrainDescription* terraindescr_;729 const Widelands::TerrainDescription* terraindescr_;
729};730};
730731
732class LuaEconomy : public LuaMapModuleClass {
733public:
734 LUNA_CLASS_HEAD(LuaEconomy);
735
736 virtual ~LuaEconomy() {
737 }
738
739 LuaEconomy() : economy_(nullptr) {
740 }
741 LuaEconomy(Widelands::Economy* economy) : economy_(economy) {
742 }
743 LuaEconomy(lua_State* L) : economy_(nullptr) {
744 report_error(L, "Cannot instantiate a 'LuaEconomy' directly!");
745 }
746
747 void __persist(lua_State* L) override;
748 void __unpersist(lua_State* L) override;
749
750 /*
751 * Properties
752 */
753
754 /*
755 * Lua methods
756 */
757 int ware_target_quantity(lua_State*);
758 int worker_target_quantity(lua_State*);
759 int set_ware_target_quantity(lua_State*);
760 int set_worker_target_quantity(lua_State*);
761
762 /*
763 * C methods
764 */
765
766protected:
767 Widelands::Economy* get() const {
768 assert(economy_ != nullptr);
769 return economy_;
770 }
771 // For persistence.
772 void set_economy_pointer(Widelands::Economy* pointer) {
773 economy_ = pointer;
774 }
775
776private:
777 Widelands::Economy* economy_;
778};
779
731#define CASTED_GET(klass) \780#define CASTED_GET(klass) \
732 Widelands::klass* get(lua_State* L, Widelands::EditorGameBase& egbase) { \781 Widelands::klass* get(lua_State* L, Widelands::EditorGameBase& egbase) { \
733 return static_cast<Widelands::klass*>(LuaMapObject::get(L, egbase, #klass)); \782 return static_cast<Widelands::klass*>(LuaMapObject::get(L, egbase, #klass)); \
@@ -904,6 +953,7 @@
904 /*953 /*
905 * Properties954 * Properties
906 */955 */
956 int get_economy(lua_State* L);
907 int get_roads(lua_State* L);957 int get_roads(lua_State* L);
908 int get_building(lua_State* L);958 int get_building(lua_State* L);
909 /*959 /*
910960
=== added file 'test/maps/lua_testsuite.wmf/scripting/geconomy.lua'
--- test/maps/lua_testsuite.wmf/scripting/geconomy.lua 1970-01-01 00:00:00 +0000
+++ test/maps/lua_testsuite.wmf/scripting/geconomy.lua 2017-03-29 16:02:45 +0000
@@ -0,0 +1,52 @@
1-- ==================================================
2-- Tests for Economy that are only useful in the Game
3-- ==================================================
4
5economy_tests = lunit.TestCase("Economy test")
6function economy_tests:test_instantiation_forbidden()
7 assert_error("Cannot instantiate", function()
8 wl.map.Economy()
9 end)
10end
11
12function economy_tests:test_ware_target_quantity()
13
14 -- Get the economy off a flag
15 local sf = map:get_field(10, 10)
16 local hq = player1:place_building("barbarians_headquarters", sf, false, true)
17 local hq_flag = hq.flag
18 local eco = hq_flag.economy
19
20 -- Test illegal parameters
21 assert_error("Nonexisting ware",function() eco:ware_target_quantity("foobar") end)
22 assert_error("Quantity for nonexisting ware",function() eco:worker_target_quantity("foobar", 1) end)
23 assert_error("Negative ware quantity",function() eco:set_ware_target_quantity("log", -1) end)
24
25 -- Now set and confirm ware quantity
26 quantity = eco:ware_target_quantity("log")
27 quantity = quantity + 1
28 eco:set_ware_target_quantity("log", quantity)
29 assert_equal(quantity, eco:ware_target_quantity("log"))
30
31 hq_flag:remove()
32end
33
34function economy_tests:test_worker_target_quantity()
35 -- Get the economy off a flag
36 local sf = map:get_field(10, 10)
37 local hq = player1:place_building("barbarians_headquarters", sf, false, true)
38 local hq_flag = hq.flag
39 local eco = hq_flag.economy
40
41 -- Test illegal parameters
42 assert_error("Nonexisting worker",function() eco:worker_target_quantity("foobar") end)
43 assert_error("Quantity for nonexisting worker",function() eco:worker_target_quantity("foobar", 1) end)
44 assert_error("Negative worker quantity",function() eco:set_worker_target_quantity("barbarians_soldier", -1) end)
45
46 -- Now set and confirm worker quantity
47 quantity = eco:worker_target_quantity("barbarians_soldier")
48 quantity = quantity + 1
49 eco:set_worker_target_quantity("barbarians_soldier", quantity)
50 assert_equal(quantity, eco:worker_target_quantity("barbarians_soldier"))
51 hq_flag:remove()
52end
053
=== modified file 'test/maps/lua_testsuite.wmf/scripting/init.lua'
--- test/maps/lua_testsuite.wmf/scripting/init.lua 2016-04-12 07:35:33 +0000
+++ test/maps/lua_testsuite.wmf/scripting/init.lua 2017-03-29 16:02:45 +0000
@@ -40,6 +40,7 @@
40if not wl.editor then40if not wl.editor then
41 include "map:scripting/game.lua"41 include "map:scripting/game.lua"
4242
43 include "map:scripting/geconomy.lua"
43 include "map:scripting/gplayer.lua"44 include "map:scripting/gplayer.lua"
44 include "map:scripting/gfield.lua"45 include "map:scripting/gfield.lua"
45 include "map:scripting/gplr_access.lua"46 include "map:scripting/gplr_access.lua"

Subscribers

People subscribed via source and target branches

to status/vote changes: