[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3971. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/428541931. Appveyor build 3769. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3769. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3960. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/428009824. Appveyor build 3758. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3758. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3949. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/427562201. Appveyor build 3747. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3747. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
That's a missing include. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3940. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/427333771. Appveyor build 3738. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3738. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
appveyor failed on a reason different to the issue with glbinding. something with throw Wexcepetion which I don't understand -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3933. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/427067915. Appveyor build 3731. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3731. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
this will fail in CI, as it does not contain any of the late fixes for them -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 3887. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/424046125. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Thanks for the review :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Review: Approve code Code LGTM. Nice catch :) The problem was that the WorkerDescr fetched the table, then took only value [1], then discarded the table, so [2] was unused. Then it fetched the table anew, used only key [2], and discarded the table, so value [1] was unused. A table always prints warnings about unused keys when being deleted. get_vector() is a very elegant workaround for this :) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Diff is looking okay and the error messages are gone. [Disclaimer: I don't know much about the lua interface.] Basically the error message are avoided by not using get_int() anymore but using array_entries(). Is there a reason against fixing (?) get_int() (and others) so the usage of the table key is stored in the set accessed_keys_ as well, as is done inside array_entries() ? From the code it seems as if using any method except array_entries() will lead to the error message, which seems strange to me. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
Continuous integration builds have changed state: Travis build 3887. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/424046125. Appveyor build 3685. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1772168_unused_key_in_luatable-3685. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into
lp:widelands.
Commit message:
New function get_vector() in LuaTable for reading hotspots. This fixes 'Error:
Unused key "1"/"2" in Lua table.' for loading ware hotspots.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1772168 in widelands: "Some 'Error: Unused key "1"/"2" in Lua table."
https://bugs.launchpad.net/widelands/+bug/1772168
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1772168-unused-key-in-luatable/+merge/354202
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1772168-unused-key-in-luatable into
lp:widelands.
=== modified file 'src/graphic/animation.cc'
--- src/graphic/animation.cc 2018-07-08 16:10:50 +
+++ src/graphic/animation.cc 2018-09-03 17:58:19 +
@@ -42,17 +42,6 @@
#include "sound/sound_handler.h"
namespace {
-
-// Parses an array { 12, 23 } into a point.
-void get_point(const LuaTable& table, Vector2i* p) {
- std::vector pts = table.array_entries();
- if (pts.size() != 2) {
- throw wexception("Expected 2 entries, but got %" PRIuS ".", pts.size());
- }
- p->x = pts[0];
- p->y = pts[1];
-}
-
/**
* Implements the Animation interface for an animation that is unpacked on disk, that
* is every frame and every pc color frame is an singular file on disk.
@@ -107,10 +96,8 @@
};
NonPackedAnimation::NonPackedAnimation(const LuaTable& table)
- : frametime_(FRAME_LENGTH), hasplrclrs_(false), scale_(1), play_once_(false) {
+ : frametime_(FRAME_LENGTH), hotspot_(table.get_vector("hotspot")), hasplrclrs_(false), scale_(1), play_once_(false) {
try {
- get_point(*table.get_table("hotspot"), _);
-
if (table.has_key("sound_effect")) {
std::unique_ptr sound_effects = table.get_table("sound_effect");
=== modified file 'src/logic/map_objects/tribes/worker_descr.cc'
--- src/logic/map_objects/tribes/worker_descr.cc 2018-05-07 05:35:32 +
+++ src/logic/map_objects/tribes/worker_descr.cc 2018-09-03 17:58:19 +
@@ -40,8 +40,7 @@
const EditorGameBase& egbase)
: BobDescr(init_descname, init_type, MapObjectDescr::OwnerType::kTribe, table),
ware_hotspot_(table.has_key("ware_hotspot") ?
- Vector2i(table.get_table("ware_hotspot")->get_int(1),
- table.get_table("ware_hotspot")->get_int(2)) :
+ table.get_vector("ware_hotspot") :
Vector2i(0, 15)),
default_target_quantity_(table.has_key("default_target_quantity") ?
table.get_int("default_target_quantity") :
=== modified file 'src/scripting/lua_table.h'
--- src/scripting/lua_table.h 2018-04-07 16:59:00 +
+++ src/scripting/lua_table.h 2018-09-03 17:58:19 +
@@ -27,6 +27,7 @@
#include
+#include "base/vector.h"
#include "scripting/lua.h"
#include "scripting/lua_coroutine.h"
#include "scripting/lua_errors.h"
@@ -112,6 +113,19 @@
return rv;
}
+ // Parses a Lua subtable into a Vector2i or Vector2f
+ template Vector2 get_vector(const KeyType& key) const {
+ Vector2 result = Vector2::zero();
+ std::unique_ptr table(get_table(key));
+ std::vector pts = table->array_entries();
+ if (pts.size() != 2) {
+ throw wexception("Expected 2 entries, but got %" PRIuS ".", pts.size());
+ }
+ result.x = pts[0];
+ result.y = pts[1];
+ return result;
+ }
+
template std::unique_ptr get_table(const KeyType& key) const {
get_existing_table_value(key);
if (!lua_istable(L_, -1)) {
___
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
