[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1480961_configure_economy into lp:widelands
Continuous integration builds have changed state: Travis build 1004. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/122593739. Appveyor build 837. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1480961_configure_economy-837. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1480961_configure_economy/+merge/291669 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug_1480961_configure_economy 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/test-ngettext into lp:widelands
> Maybe we could write a wrapper that will map floating point numbers to the > appropriate integer numbers for the languages in our list, and then use those > integers to call ngettext? That wouldn't work for Slovak, because floats need their own, different form. There is nothing to map to. > The alternative would be not to use any placeholders in these strings - that > would increase the translation effort though. For example, we have 6 different > types of Iron Mine, which currently can use the same string for their > productivity texts. So, without the placeholders we get 6x the translation > effort, plus if the calculation should change in the future, the strings would > need to be retranslated. That doesn't sound like it's worth it, IMO, especially when we can simply use the symbol of the second. I'm not completely opposed to it, though -- it does get the job done. -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/test-ngettext. ___ 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/test-ngettext into lp:widelands
The actual rule for Scottish Gaelic is to discard everything after the ., because we say "x seconds point y" for x.y seconds. Not having things correct for Slovak doesn't please me though. Maybe we could write a wrapper that will map floating point numbers to the appropriate integer numbers for the languages in our list, and then use those integers to call ngettext? The alternative would be not to use any placeholders in these strings - that would increase the translation effort though. For example, we have 6 different types of Iron Mine, which currently can use the same string for their productivity texts. So, without the placeholders we get 6x the translation effort, plus if the calculation should change in the future, the strings would need to be retranslated. -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/test-ngettext. ___ 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_1480961_configure_economy into lp:widelands
Not tested yet, but code LGTM. @Tibor: does this affect the AI in any way? -- https://code.launchpad.net/~widelands-dev/widelands/bug_1480961_configure_economy/+merge/291669 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug_1480961_configure_economy 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_1480961_configure_economy into lp:widelands
kaputtnik has proposed merging
lp:~widelands-dev/widelands/bug_1480961_configure_economy into lp:widelands.
Commit message:
Hide wares from configure economy which does not need prerequisites and
therefor are produced 'endless'
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1480961 in widelands: "Buildings produce more than set in "configure
economy""
https://bugs.launchpad.net/widelands/+bug/1480961
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug_1480961_configure_economy/+merge/291669
Solves bug 1480961
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug_1480961_configure_economy into lp:widelands.
=== modified file 'data/tribes/wares/blackroot/init.lua'
--- data/tribes/wares/blackroot/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/blackroot/init.lua 2016-04-12 18:32:13 +
@@ -7,9 +7,7 @@
descname = pgettext("ware", "Blackroot"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- atlanteans = 20
- },
+ default_target_quantity = {},
preciousness = {
atlanteans = 10
},
=== modified file 'data/tribes/wares/corn/init.lua'
--- data/tribes/wares/corn/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/corn/init.lua 2016-04-12 18:32:13 +
@@ -7,9 +7,7 @@
descname = pgettext("ware", "Corn"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- atlanteans = 25
- },
+ default_target_quantity = {},
preciousness = {
atlanteans = 12
},
=== modified file 'data/tribes/wares/fish/init.lua'
--- data/tribes/wares/fish/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/fish/init.lua 2016-04-12 18:32:13 +
@@ -7,11 +7,7 @@
descname = pgettext("ware", "Fish"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- atlanteans = 20,
- barbarians = 20,
- empire = 10
- },
+ default_target_quantity = {},
preciousness = {
atlanteans = 4,
barbarians = 3,
=== modified file 'data/tribes/wares/granite/init.lua'
--- data/tribes/wares/granite/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/granite/init.lua 2016-04-12 18:32:13 +
@@ -7,11 +7,7 @@
descname = pgettext("ware", "Granite"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- atlanteans = 20,
- barbarians = 20,
- empire = 30
- },
+ default_target_quantity = {},
preciousness = {
atlanteans = 5,
barbarians = 5,
=== modified file 'data/tribes/wares/grape/init.lua'
--- data/tribes/wares/grape/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/grape/init.lua 2016-04-12 18:32:13 +
@@ -7,9 +7,7 @@
descname = pgettext("ware", "Grape"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- empire = 20
- },
+ default_target_quantity = {},
preciousness = {
empire = 10
},
=== modified file 'data/tribes/wares/log/init.lua'
--- data/tribes/wares/log/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/log/init.lua 2016-04-12 18:32:13 +
@@ -7,11 +7,7 @@
descname = pgettext("ware", "Log"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- atlanteans = 40,
- barbarians = 40,
- empire = 40
- },
+ default_target_quantity = {},
preciousness = {
atlanteans = 14,
barbarians = 14,
=== modified file 'data/tribes/wares/marble/init.lua'
--- data/tribes/wares/marble/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/marble/init.lua 2016-04-12 18:32:13 +
@@ -7,9 +7,7 @@
descname = pgettext("ware", "Marble"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- empire = 30
- },
+ default_target_quantity = {},
preciousness = {
empire = 3
},
=== modified file 'data/tribes/wares/thatch_reed/init.lua'
--- data/tribes/wares/thatch_reed/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/thatch_reed/init.lua 2016-04-12 18:32:13 +
@@ -7,9 +7,7 @@
descname = pgettext("ware", "Thatch Reed"),
helptext_script = dirname .. "helptexts.lua",
icon = dirname .. "menu.png",
- default_target_quantity = {
- barbarians = 10
- },
+ default_target_quantity = {},
preciousness = {
barbarians = 5
},
=== modified file 'data/tribes/wares/water/init.lua'
--- data/tribes/wares/water/init.lua 2015-12-11 16:54:00 +
+++ data/tribes/wares/water/init.lua 2016-04-12 18:32:13 +
@@ -7,11 +7,7 @@
descname = pgettext("ware", "Water"),
helptext_script = dirname .. "helptexts.lua",
icon =
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/test-ngettext into lp:widelands
According to the gettext manual, "a simple gettext call with a form suitable for all values will do"[1] -- something like http://pastebin.com/raw/R2tGW9XH. The CLDR tables[2] clearly disprove this rule being universal, though. Flooring produces incorrect results for Slovak and I bet a lot of other languages too, including English. Actually, assuming the information in the CLDR table is correct (and that my understanding of it is correct), Scottish Gaelic suffers as well. I haven't really looked into it, but I think a fully-featured floating-point version of ngettext wouldn't be possible without writing our own i18n system/forking gettext. We'd probably need to extend the plural expression format and who knows if/how that would work with Transifex. I'd really like to know how other projects deal with this issue, that is if they care at all. Perhaps we'd be better off using an abbreviation or a universal unit symbol, like "%d s", for the time being. [1] http://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/Plural-forms.html (at the end of the page) [2] http://www.unicode.org/cldr/charts/latest/supplemental/language_plural_rules.html -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/test-ngettext. ___ 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/test-ngettext into lp:widelands
An example: "If the food supply is steady, this mine can produce iron ore in %s on average." %s is replaced by "%d second(s)", and %d can become floating point. The floor is what we have always had implicitly (and it also happens to be what my particular language does), so there is no change in functionality. What we really want is a floating-point version of ngettext, but that will be some work. -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/test-ngettext. ___ 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/test-ngettext into lp:widelands
Review: Approve compile, regressiontest.py +/- Having more tests is always good and the code looks OK for me. test-ngettext klaus$ ./regression_test.py -b ./widelands OTOH Miroslav question is valid, where or when do we need to show someting like "You have 3.145972 Item(plurals) in you Inventory" ? -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/test-ngettext. ___ 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/test-ngettext into lp:widelands
Continuous integration builds have changed state: Travis build 1000. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/122450845. Appveyor build 833. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_test_ngettext-833. -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/test-ngettext 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/test-ngettext into lp:widelands
I'm not a fan of flooring the number in our C++ code. Passing a non-integer value to ngettext from Lua should result in an error (no idea what crashes you're talking about), IMO. Where do we need to pass floats? -- https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/test-ngettext 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-1545243-plnum-lua into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1545243-plnum-lua into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1545243-plnum-lua. ___ 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-1545243-plnum-lua into lp:widelands
Excellent, thanks for the review and testing! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1545243-plnum-lua. ___ 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-1545243-plnum-lua into lp:widelands
Review: Approve review/compile/regression testst, playing OK, all fine now. I played a savegame for perhaps 15 Minutes, looked all fine -- https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1545243-plnum-lua. ___ 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/test-ngettext into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/test-ngettext into
lp:widelands.
Commit message:
Created test for lua gettext functions.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/test-ngettext/+merge/291587
We missed fixing ngettext when updating eris, so I've created a test.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/test-ngettext into lp:widelands.
=== added file 'test/maps/lua_testsuite.wmf/scripting/gettext.lua'
--- test/maps/lua_testsuite.wmf/scripting/gettext.lua 1970-01-01 00:00:00 +
+++ test/maps/lua_testsuite.wmf/scripting/gettext.lua 2016-04-12 07:40:22 +
@@ -0,0 +1,26 @@
+test_descr = lunit.TestCase("Gettext Test")
+
+-- Since we run the test suite in English, this will only test
+-- whether gettext functions crash or not
+
+function test_descr:test_gettext()
+ set_textdomain("tribes")
+ assert_equal("foo", _"foo")
+ assert_equal("foo", _("foo"))
+ assert_equal(_"Carrier", _("Carrier"))
+end
+
+function test_descr:test_ngettext()
+ assert_equal("foo", ngettext("foo", "bar", 1))
+ -- TODO(GunChleoc): Just using the floor here is wrong for English,
+ -- but an edge case as compared to other languages that need it here.
+ -- ngettext doesn't actually support floating point numbers,
+ -- so we might want to create our own rules off the CLDR some time.
+ assert_equal("foo", ngettext("foo", "bar", 1.5))
+ assert_equal("bar", ngettext("foo", "bar", 1000))
+ assert_equal("bar", ngettext("foo", "bar", 1000.5))
+end
+
+function test_descr:test_pgettext()
+ assert_equal("bar", pgettext("foo", "bar"))
+end
=== modified file 'test/maps/lua_testsuite.wmf/scripting/init.lua'
--- test/maps/lua_testsuite.wmf/scripting/init.lua 2016-01-25 18:50:54 +
+++ test/maps/lua_testsuite.wmf/scripting/init.lua 2016-04-12 07:40:22 +
@@ -23,6 +23,7 @@
-- =
include "map:scripting/egbase.lua"
+include "map:scripting/gettext.lua"
include "map:scripting/math_random.lua"
include "map:scripting/string_bformat.lua"
include "map:scripting/path.lua"
___
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-1545243-plnum-lua into lp:widelands
Review: Resubmit This should be all fixed now. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1545243-plnum-lua. ___ 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-1545243-plnum-lua into lp:widelands
> * When setting some options I get:
>
> [] Section [global], key 'depth' not used (did you spell the name correctly?)
> [] Section [global], key 'ui_font' not used (did you spell the name
> correctly?)
> [] Section [global], key 'speed_of_new_game' not used (did you spell the name
> correctly?)
> [] Section [global], key 'remove_replays' not used (did you spell the name
> correctly?)
> [] Section [global], key 'remove_syncstreams' not used (did you spell the name
> correctly?)
> [] Section [global], key 'opengl' not used (did you spell the name correctly?)
>
> But that may work as intended.
When you get something that's seemingly unrelated, test it on trunk as well - I
bet it's the same.
> * OK, I cannot use an old replay, as of UnhandledVersionError
We broke savegame compatibility with
http://bazaar.launchpad.net/~widelands-dev/widelands/trunk/revision/7954
> * Got a crash in the regression tests, will add info for this later
This definitely needs fixing.
I also replied to your diff comments.
Diff comments:
>
> === modified file 'src/editor/map_generator.cc'
> --- src/editor/map_generator.cc 2016-04-06 09:23:04 +
> +++ src/editor/map_generator.cc 2016-04-11 07:07:30 +
> @@ -130,9 +130,9 @@
> const auto set_resource_helper = [this, , _description,
> ] (
> const uint32_t random_value, const int valid_resource_index) {
> const DescriptionIndex res_idx =
> terrain_description.get_valid_resource(valid_resource_index);
> - const uint32_t max_amount =
> world.get_resource(res_idx)->max_amount();
> - uint8_t res_val = static_cast(random_value /
> (kMaxElevation / max_amount));
> - res_val *= static_cast(map_info_.resource_amount) + 1;
> + const ResourceAmount max_amount =
> world.get_resource(res_idx)->max_amount();
> + ResourceAmount res_val =
> static_cast(random_value / (kMaxElevation / max_amount));
> + res_val *=
> static_cast(map_info_.resource_amount) + 1;
If the old casts would have limited the values, that would have been a bug IMO.
So, no problem if the calculation changed here.
> res_val /= 3;
> if (map_.is_resource_valid(world, fc, res_idx)) {
> map_.initialize_resources(fc, res_idx, res_val);
>
> === modified file 'src/editor/tools/set_resources_tool.cc'
> --- src/editor/tools/set_resources_tool.cc2016-04-06 09:23:04 +
> +++ src/editor/tools/set_resources_tool.cc2016-04-11 07:07:30 +
> @@ -68,11 +68,9 @@
> EditorActionArgs* args,
> Widelands::Map* map) {
> for (const auto & res : args->original_resource) {
> - int32_t amount = res.amount;
> - int32_t max_amount =
> world.get_resource(args->current_resource)->max_amount();
> + Widelands::ResourceAmount amount = res.amount;
> + Widelands::ResourceAmount max_amount =
> world.get_resource(args->current_resource)->max_amount();
>
> - if (amount < 0)
> - amount = 0;
These lines should not have been deleted - I'll add them back in.
> if (amount > max_amount)
> amount = max_amount;
>
>
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc 2016-04-06 09:23:04 +
> +++ src/logic/map.cc 2016-04-11 07:07:30 +
> @@ -1869,7 +1869,7 @@
> }
>
> bool Map::is_resource_valid
> - (const Widelands::World& world, const TCoords& c,
> int32_t const curres)
> + (const Widelands::World& world, const TCoords& c,
> DescriptionIndex curres)
It's a basic datatype, so const doesn't matter in the interface here. We
actually have a codecheck rule in place to weed them out, but it doesn't catch
all instances.
> {
> if (curres == Widelands::kNoResource)
> return true;
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1545243-plnum-lua/+merge/291481
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1545243-plnum-lua.
___
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
