Re: [Widelands-dev] [Merge] lp:~franku/widelands-website/devs_and_locales_list into lp:widelands-website
Review: Approve
I can't test this, but code LGTM - just some small formatting nits.
I also found a typo that's not in the diff:
line 86: at given position and prepaire all for wl_markdown
prepaire => prepare
Diff comments:
> === modified file 'mainpage/views.py'
> --- mainpage/views.py 2015-05-19 07:12:45 +
> +++ mainpage/views.py 2015-07-27 19:11:30 +
> @@ -51,38 +50,52 @@
> txt = ""
> transl_files = []
> transl_list = []
> -path = os.path.normpath(WIDELANDS_SVN_DIR + "txts/translators/")
> +path = os.path.normpath(WIDELANDS_SVN_DIR + "i18n/locales/")
> try:
> transl_files = os.listdir(path)
> if transl_files:
> for fname in transl_files:
> -if fname.endswith(".json"):
> +if fname.endswith(".json") :
We don't have a blank space before the : anywhere else.
> with open(path + "/" + fname,"r") as f:
> -json_data = json.load(f)["locale-translators"]
> -
> -if json_data["translator-list"] != "translator-credits":
> +json_data = json.load(f)
> +
> +try:
> +if json_data["translator-list"] !=
> "translator-credits":
> transl_list.append(json_data)
> +except KeyError:
> +transl_list = ["KeyError"]
> +
> +# Check for the other key we need
> +for d in transl_list:
> +if not "your-language-name-in-english" in d:
> +transl_list = ["KeyError"]
> +
> +# No KeyError -> Sort the list
> +if "KeyError" in transl_list:
> +txt = "Some Translator key is wrong, please contact the
> Developers. \n"
Remove blank space before \n
> +else:
> +transl_list.sort(
> key=itemgetter("your-language-name-in-english"))
Remove blank space after (
> +
> else:
> -txt = "No files for translators found!"
> +txt = "No files for translators found!\n"
> except OSError:
> -txt = txt + "Couldn't find translators directory!"
> +txt = txt + "Couldn't find translators directory!\n"
>
>
> # Get other developers, put in the translators list
> # at given position and prepaire all for wl_markdown
> try:
> -f = open(WIDELANDS_SVN_DIR + "txts/developers.json", "r")
> -json_data = json.load(f)["developers"]
> -f.close()
> -
> +with open(WIDELANDS_SVN_DIR + "txts/developers.json", "r") as f:
> +json_data = json.load(f)["developers"]
> +
> for head in json_data:
> # Add first header
> txt = txt + "##" + head["heading"] + "\n"
> -# Inserting Translators
> -if head["heading"] == "Translators":
> +# Inserting Translators if there was no error
> +if head["heading"] == "Translators" and "KeyError" not in
> transl_list:
> for values in (transl_list):
> # Add subheader for locale
> -txt = txt + "### " + values["your-language-name"] + "\n"
> +txt = txt + "### " +
> values["your-language-name-in-english"] + "\n"
> # Prepaire the names for wl_markdown
Prepaire => Prepare
> txt = txt + "* " +
> values["translator-list"].replace('\n', '\n* ') + "\n"
>
--
https://code.launchpad.net/~franku/widelands-website/devs_and_locales_list/+merge/266025
Your team Widelands Developers is subscribed to branch lp:widelands-website.
___
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/trainingsites_and_teams into lp:widelands
I incorporated the comments, I belive all of them, though I was unsure about iterators, there is a lot of places where iterators are used here... Maybe it will be reworked gradually. regression tests passed OK -- https://code.launchpad.net/~widelands-dev/widelands/trainingsites_and_teams/+merge/260517 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/trainingsites_and_teams 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:~franku/widelands-website/devs_and_locales_list into lp:widelands-website
kaputtnik has proposed merging
lp:~franku/widelands-website/devs_and_locales_list into lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1424072 in Widelands Website: "Automatically add translator credits to
authors list"
https://bugs.launchpad.net/widelands-website/+bug/1424072
For more details, see:
https://code.launchpad.net/~franku/widelands-website/devs_and_locales_list/+merge/266025
Related changes to the new translator files.
I tried to implement some checks to the keys of the translators files without
breaking the website if there are some keys wrong. Theses checks are currently
only applied to the translator keys "translator-list" and
"your-language-name-in-english" (the two keys we need for the website). If one
or noth keys are wrong, a little text is placed at the top of the "Development
Team" list.
The keys of file "developers.json" are not tested... i tried it but it seems a
bit overkill.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~franku/widelands-website/devs_and_locales_list into lp:widelands-website.
=== modified file 'mainpage/views.py'
--- mainpage/views.py 2015-05-19 07:12:45 +
+++ mainpage/views.py 2015-07-27 19:11:30 +
@@ -2,9 +2,8 @@
from django.template import RequestContext
from settings import WIDELANDS_SVN_DIR
from templatetags.wl_markdown import do_wl_markdown
-
+from operator import itemgetter
import sys
-import re
import json
import os
import os.path
@@ -51,38 +50,52 @@
txt = ""
transl_files = []
transl_list = []
-path = os.path.normpath(WIDELANDS_SVN_DIR + "txts/translators/")
+path = os.path.normpath(WIDELANDS_SVN_DIR + "i18n/locales/")
try:
transl_files = os.listdir(path)
if transl_files:
for fname in transl_files:
-if fname.endswith(".json"):
+if fname.endswith(".json") :
with open(path + "/" + fname,"r") as f:
-json_data = json.load(f)["locale-translators"]
-
-if json_data["translator-list"] != "translator-credits":
+json_data = json.load(f)
+
+try:
+if json_data["translator-list"] != "translator-credits":
transl_list.append(json_data)
+except KeyError:
+transl_list = ["KeyError"]
+
+# Check for the other key we need
+for d in transl_list:
+if not "your-language-name-in-english" in d:
+transl_list = ["KeyError"]
+
+# No KeyError -> Sort the list
+if "KeyError" in transl_list:
+txt = "Some Translator key is wrong, please contact the Developers. \n"
+else:
+transl_list.sort( key=itemgetter("your-language-name-in-english"))
+
else:
-txt = "No files for translators found!"
+txt = "No files for translators found!\n"
except OSError:
-txt = txt + "Couldn't find translators directory!"
+txt = txt + "Couldn't find translators directory!\n"
# Get other developers, put in the translators list
# at given position and prepaire all for wl_markdown
try:
-f = open(WIDELANDS_SVN_DIR + "txts/developers.json", "r")
-json_data = json.load(f)["developers"]
-f.close()
-
+with open(WIDELANDS_SVN_DIR + "txts/developers.json", "r") as f:
+json_data = json.load(f)["developers"]
+
for head in json_data:
# Add first header
txt = txt + "##" + head["heading"] + "\n"
-# Inserting Translators
-if head["heading"] == "Translators":
+# Inserting Translators if there was no error
+if head["heading"] == "Translators" and "KeyError" not in transl_list:
for values in (transl_list):
# Add subheader for locale
-txt = txt + "### " + values["your-language-name"] + "\n"
+txt = txt + "### " + values["your-language-name-in-english"] + "\n"
# Prepaire the names for wl_markdown
txt = txt + "* " + values["translator-list"].replace('\n', '\n* ') + "\n"
___
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/authors_and_languages into lp:widelands
Added a small change in the JSON files to make things easier for the website side of things. -- https://code.launchpad.net/~widelands-dev/widelands/authors_and_languages/+merge/265892 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/authors_and_languages. ___ 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-653308 into lp:widelands
I don't know why a delete was necessary - maybe it was just old code? 2 of the methods now return unique_ptr. add_text is trickier and was giving me segfaults (probably because it is being used directly as well as for the member object), so I just added a todo comment for it. We should run clang-format on this once we're done. -- https://code.launchpad.net/~widelands-dev/widelands/bug-653308/+merge/265931 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-653308. ___ 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-1357510 into lp:widelands
Review: Needs Fixing
Only reviewed the code that I did not write. Some comments inlined.
Diff comments:
>
> === modified file 'src/editor/ui_menus/editor_main_menu_random_map.cc'
> --- src/editor/ui_menus/editor_main_menu_random_map.cc2014-11-30
> 18:49:38 +
> +++ src/editor/ui_menus/editor_main_menu_random_map.cc2015-07-27
> 11:00:34 +
> @@ -504,12 +504,13 @@
> << "ID = " << m_idEditbox->text() << "\n";
>
> MapGenerator gen(map, mapInfo, egbase);
> - map.create_empty_map(egbase.world(),
> - mapInfo.w,
> - mapInfo.h,
> - _("No Name"),
> -
> g_options.pull_section("global").get_string("realname", _("Unknown")),
> - sstrm.str().c_str());
> + map.create_empty_map(
strange formatting
> + egbase.world(),
> + mapInfo.w,
> + mapInfo.h,
> + _("No Name"),
> +
> g_options.pull_section("global").get_string("realname", pgettext("map_name",
> "Unknown")),
> + sstrm.str().c_str());
> loader.step(_("Generating random map..."));
> gen.create_random_map();
>
>
> === modified file 'src/editor/ui_menus/editor_player_menu.cc'
> --- src/editor/ui_menus/editor_player_menu.cc 2015-06-05 19:13:06 +
> +++ src/editor/ui_menus/editor_player_menu.cc 2015-07-27 11:00:34 +
> @@ -207,7 +207,7 @@
> number += '0' + nr_players_10;
> number += '0' + nr_players % 10;
> /** TRANSLATORS: Default player name, e.g. Player 1 */
> - const std::string name = (boost::format(_("Player %s")) %
> number).str();
> + const std::string name = (boost::format(_("Player %u")) %
> nr_players).str();
make sure that nr_players is not a char/uint8_t. Otherwise this will not work
properly iirc. Might need a static_cast() in that case
> map.set_scenario_player_name(nr_players, name);
> }
> map.set_scenario_player_tribe(nr_players, m_tribes[0]);
>
> === modified file 'src/scripting/persistence.cc'
> --- src/scripting/persistence.cc 2015-01-31 16:03:59 +
> +++ src/scripting/persistence.cc 2015-07-27 11:00:34 +
> @@ -165,7 +165,7 @@
> "package", "pairs", "pcall", "print", "rawequal", "rawget", "rawset",
> "rawlen", "require", "select", "setfenv", "setmetatable", "table",
> "tonumber", "tostring", "type", "unpack", "wl", "xpcall", "string",
> - "_", "set_textdomain", "get_build_id", "coroutine.yield", "ngettext",
> + "_", "set_textdomain", "get_build_id", "coroutine.yield", "ngettext",
> "pgettext",
The ordering of these matter. Please only append at the end, i.e. after "path"
before nullptr
> "include", "path", nullptr
> };
>
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1357510/+merge/265949
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1357510.
___
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-653308 into lp:widelands
Review: Needs Information
I am unsure about this change now. Most UI elements are owned by their parents.
Why did we need the delete before in the first place? More comments inlined.
Diff comments:
>
> === modified file 'src/wui/attack_box.cc'
> --- src/wui/attack_box.cc 2015-03-30 08:45:09 +
> +++ src/wui/attack_box.cc 2015-07-27 10:01:25 +
> @@ -119,31 +113,43 @@
> return *button;
> }
>
> +/*
> + * Update available soldiers
> + */
> +void AttackBox::think() {
> + int32_t gametime = player_->egbase().get_gametime();
can still be const
> + if ((gametime - lastupdate_) > kUpdateTimeInGametimeMs) {
> + update_attack();
> + lastupdate_ = gametime;
> + }
> +}
> +
> void AttackBox::update_attack() {
> - assert(m_slider_soldiers);
> - assert(m_text_soldiers);
> - assert(m_less_soldiers);
> - assert(m_add_soldiers);
> + assert(soldiers_slider_.get());
> + assert(soldiers_text_.get());
> + assert(less_soldiers_.get());
> + assert(more_soldiers_.get());
>
> int32_t max_attackers = get_max_attackers();
>
> - if (m_slider_soldiers->get_max_value() != max_attackers)
> - m_slider_soldiers->set_max_value(max_attackers);
> + if (soldiers_slider_->get_max_value() != max_attackers) {
> + soldiers_slider_->set_max_value(max_attackers);
> + }
>
> - m_slider_soldiers->set_enabled(max_attackers > 0);
> - m_add_soldiers->set_enabled(max_attackers >
> m_slider_soldiers->get_value());
> - m_less_soldiers ->set_enabled(m_slider_soldiers->get_value() > 0);
> + soldiers_slider_->set_enabled(max_attackers > 0);
> + more_soldiers_->set_enabled(max_attackers >
> soldiers_slider_->get_value());
> + less_soldiers_ ->set_enabled(soldiers_slider_->get_value() > 0);
>
> /** TRANSLATORS: %1% of %2% soldiers. Used in Attack box. */
> - m_text_soldiers->set_text((boost::format(_("%1% / %2%"))
> - %
> m_slider_soldiers->get_value()
> + soldiers_text_->set_text((boost::format(_("%1% / %2%"))
> + %
> soldiers_slider_->get_value()
> %
> max_attackers).str());
>
> - m_add_soldiers->set_title(std::to_string(max_attackers));
> + more_soldiers_->set_title(std::to_string(max_attackers));
> }
>
> void AttackBox::init() {
> - assert(m_node);
> + assert(node_coordinates_);
>
> uint32_t max_attackers = get_max_attackers();
>
> @@ -166,42 +172,42 @@
> const std::string attack_string =
> (boost::format(_("%1% / %2%")) % (max_attackers > 0 ? 1
> : 0) % max_attackers).str();
>
> - m_text_soldiers =
> + soldiers_text_.reset(
on second look that looks weird Can you doublecheck that add_text really
passes ownership? Maybe the delete's were wrong before?
If it passes ownership, it should directly return a unique_ptr<>. If that is a
bigger change, add a TODO so that it is at least documented. Right now it seems
to pass a reference - that is very wrong for passing ownership.
> &add_text(columnbox, attack_string, UI::Box::AlignCenter,
>UI::g_fh1->fontset().serif(),
> - UI_FONT_SIZE_ULTRASMALL);
> + UI_FONT_SIZE_ULTRASMALL));
>
> - m_slider_soldiers =
> + soldiers_slider_.reset(
> &add_slider
> (columnbox,
>100, 10,
>0, max_attackers, max_attackers > 0 ? 1 : 0,
>"pics/but2.png",
> - _("Number of soldiers"));
> + _("Number of soldiers")));
>
> -
> m_slider_soldiers->changed.connect(boost::bind(&AttackBox::update_attack,
> this));
> - m_add_soldiers =
> +
> soldiers_slider_->changed.connect(boost::bind(&AttackBox::update_attack,
> this));
> + more_soldiers_.reset(
> &add_button
> (linebox,
>std::to_string(max_attackers),
>&AttackBox::send_more_soldiers,
> - _("Send more soldiers"));
> + _("Send more soldiers")));
>
> - m_slider_soldiers->set_enabled(max_attackers > 0);
> - m_add_soldiers ->set_enabled(max_attackers > 0);
> - m_less_soldiers ->set_enabled(max_attackers > 0);
> + soldiers_slider_->set_enabled(max_attackers > 0);
> + more_soldiers_ ->set_enabled(max_attackers > 0);
> + less_soldiers_ ->set_enabled(max_attackers > 0);
> }
>
> void AttackBox::send_less_soldiers() {
> - assert(m_slider_soldiers);
> - m_slider_soldiers->set_value(m_slider_soldiers->get_value() - 1);
> + assert(soldiers_slider_.get());
> +
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1357510 into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1357510 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1357510 in widelands: "Integrate the use of pgettext from the gettext
library in C++"
https://bugs.launchpad.net/widelands/+bug/1357510
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1357510/+merge/265949
Added full gettext support to C++. Added support for pgettext in Lua scripts.
Collecting Lua strings for the .pot files only works for string literals and
multiline strings, just like with ngettext. It does not work for functions.
This works, also for multiline strings:
pgettext("hello", "world")
pgettext("hello" .. "world", "world")
This will not work:
pgettext(some_function_call(), "blub")
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1357510 into lp:widelands.
=== modified file 'src/ai/computer_player.cc'
--- src/ai/computer_player.cc 2015-02-05 09:08:19 +
+++ src/ai/computer_player.cc 2015-07-27 11:00:34 +
@@ -37,7 +37,7 @@
struct EmptyAIImpl : Implementation {
/** TRANSLATORS: This is the name of an AI used in the game setup screens */
- EmptyAIImpl() {name = _("None");}
+ EmptyAIImpl() {name = pgettext("ai_name", "None");}
ComputerPlayer * instantiate
(Widelands::Game & g, Widelands::PlayerNumber const pid) const override
{
=== modified file 'src/ai/defaultai.h'
--- src/ai/defaultai.h 2015-05-05 20:00:21 +
+++ src/ai/defaultai.h 2015-07-27 11:00:34 +
@@ -113,7 +113,7 @@
struct AggressiveImpl : public ComputerPlayer::Implementation {
AggressiveImpl() {
/** TRANSLATORS: This is the name of an AI used in the game setup screens */
- name = _("Aggressive");
+ name = pgettext("ai_name", "Aggressive");
}
ComputerPlayer* instantiate(Widelands::Game& game,
Widelands::PlayerNumber const p) const override {
@@ -124,7 +124,7 @@
struct NormalImpl : public ComputerPlayer::Implementation {
NormalImpl() {
/** TRANSLATORS: This is the name of an AI used in the game setup screens */
- name = _("Normal");
+ name = pgettext("ai_name", "Normal");
}
ComputerPlayer* instantiate(Widelands::Game& game,
Widelands::PlayerNumber const p) const override {
@@ -135,7 +135,7 @@
struct DefensiveImpl : public ComputerPlayer::Implementation {
DefensiveImpl() {
/** TRANSLATORS: This is the name of an AI used in the game setup screens */
- name = _("Defensive");
+ name = pgettext("ai_name", "Defensive");
}
ComputerPlayer* instantiate(Widelands::Game& game,
Widelands::PlayerNumber const p) const override {
=== modified file 'src/base/CMakeLists.txt'
--- src/base/CMakeLists.txt 2014-12-06 12:22:35 +
+++ src/base/CMakeLists.txt 2015-07-27 11:00:34 +
@@ -28,6 +28,7 @@
DEPENDS
base_log
base_macros
+third_party_gettext
)
wl_library(base_geometry
=== modified file 'src/base/i18n.h'
--- src/base/i18n.h 2015-01-30 23:10:35 +
+++ src/base/i18n.h 2015-07-27 11:00:34 +
@@ -24,7 +24,7 @@
#include
#include
-#include // for ngettext.
+#include "third_party/gettext/gettext.h" // For ngettext and pgettext.
#include "base/macros.h"
#include "config.h"
=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc 2015-07-26 09:25:51 +
+++ src/editor/editorinteractive.cc 2015-07-27 11:00:34 +
@@ -599,8 +599,9 @@
editor.world(),
64,
64,
+ /** TRANSLATORS: Default name for new map */
_("No Name"),
- g_options.pull_section("global").get_string("realname", _("Unknown")));
+ g_options.pull_section("global").get_string("realname", pgettext("map_name", "Unknown")));
load_all_tribes(&editor, &loader_ui);
=== modified file 'src/editor/tools/editor_info_tool.cc'
--- src/editor/tools/editor_info_tool.cc 2015-02-24 13:51:38 +
+++ src/editor/tools/editor_info_tool.cc 2015-07-27 11:00:34 +
@@ -99,7 +99,7 @@
const Widelands::TerrainDescription& ter = world.terrain_descr(
center.triangle.t == Widelands::TCoords<>::D ? tf.terrain_d() : tf.terrain_r());
- buf += "• " + (boost::format(_("Name: %s")) % ter.descname()).str() + "\n";
+ buf += "• " + (boost::format(pgettext("terrain_name", "Name: %s")) % ter.descname()).str() + "\n";
Widelands::TerrainDescription::Type terrain_is = ter.get_is();
std::vector terrain_is_strings;
@@ -144,7 +144,7 @@
// *** Map info
buf += std::string("\n") + _("Map:") + "\n";
- buf += "• " + (boost::format(_("Name: %s")) % map.get_name().c_str()).str() + "\n";
+ buf += "• " + (boost::format(pgettext("map_name", "Name: %s")) % map.get_name().c_str()).str() + "\n";
buf += "• " + (boost::format(_("Size: %1$ix%2$i"))
% map.get_width() % map.get_height()).str() + "\n";
=== modified
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1326395 into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1326395 into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1326395/+merge/265905 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1326395. ___ 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-653308 into lp:widelands
Thanks for the comments - all implemented. I also fixed up the building statistics, because that's where I stole the code from ;) -- https://code.launchpad.net/~widelands-dev/widelands/bug-653308/+merge/265931 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-653308. ___ 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/string-fixes into lp:widelands
In revision 7492,the change "Working radius" -> "Work area" in the file tribes/scripting/format_help.lua should be revised. The string prints the work area radius, so the original text "Working radius" seems more appropriate. On 27 July 2015 at 11:10, wrote: > The proposal to merge lp:~widelands-dev/widelands/string-fixes into > lp:widelands has been updated. > > Status: Needs review => Merged > > For more details, see: > > https://code.launchpad.net/~widelands-dev/widelands/string-fixes/+merge/265906 > -- > You are subscribed to branch lp:widelands. > -- https://code.launchpad.net/~widelands-dev/widelands/string-fixes/+merge/265906 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/string-fixes. ___ 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-1326395 into lp:widelands
Review: Approve -- https://code.launchpad.net/~widelands-dev/widelands/bug-1326395/+merge/265905 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1326395. ___ 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/string-fixes into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/string-fixes into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/string-fixes/+merge/265906 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/string-fixes. ___ 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/string-fixes into lp:widelands
Review: Approve Gonna merge. -- https://code.launchpad.net/~widelands-dev/widelands/string-fixes/+merge/265906 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/string-fixes. ___ 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-653308 into lp:widelands
Review: Needs Fixing
couple of nits not related to your changes. But while you are digging through
the code
Diff comments:
> === modified file 'src/ui_basic/slider.cc'
> --- src/ui_basic/slider.cc2014-11-27 12:02:08 +
> +++ src/ui_basic/slider.cc2015-07-27 07:23:23 +
> @@ -113,8 +113,10 @@
> */
> void Slider::set_max_value(int32_t new_max) {
> assert(m_min_value <= new_max);
> - if (m_max_value != new_max)
> + if (m_max_value != new_max) {
> + calc_cursor_pos();
while you around that code: calculate_cursor_position()
> update();
> + }
> m_max_value = new_max;
> set_value(m_value);
> }
>
> === modified file 'src/wui/attack_box.cc'
> --- src/wui/attack_box.cc 2015-03-30 08:45:09 +
> +++ src/wui/attack_box.cc 2015-07-27 07:23:23 +
> @@ -30,6 +30,8 @@
> #include "graphic/text_constants.h"
> #include "logic/soldier.h"
>
> +constexpr int32_t kUpdateTime = 1000; // 1 second, gametime
kUpdateTimeInGametimeMs
> +
> AttackBox::AttackBox
> (UI::Panel * parent,
>Widelands::Player * player,
> @@ -44,7 +46,9 @@
> m_node(target),
> m_slider_soldiers(nullptr),
> m_text_soldiers(nullptr),
> - m_add_soldiers(nullptr)
> + m_less_soldiers(nullptr),
> + m_add_soldiers(nullptr),
consistency: m_[add|subtract]_soldiers or m_[more|less]_soldiers.
> + lastupdate_(0)
> {
> init();
> }
> @@ -52,6 +56,7 @@
> AttackBox::~AttackBox() {
> delete m_slider_soldiers;
> delete m_text_soldiers;
> + delete m_less_soldiers;
make these into unique_ptrs? naked deletes are so 80ths.
> delete m_add_soldiers;
> }
>
> @@ -119,6 +124,17 @@
> return *button;
> }
>
> +/*
> + * Update available soldiers
> + */
> +void AttackBox::think() {
> + int32_t const gametime = m_pl->egbase().get_gametime();
const
> + if ((gametime - lastupdate_) > kUpdateTime) {
> + update_attack();
> + lastupdate_ = gametime;
> + }
> +}
> +
> void AttackBox::update_attack() {
> assert(m_slider_soldiers);
> assert(m_text_soldiers);
--
https://code.launchpad.net/~widelands-dev/widelands/bug-653308/+merge/265931
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-653308.
___
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/authors_and_languages into lp:widelands
Review: Approve -- https://code.launchpad.net/~widelands-dev/widelands/authors_and_languages/+merge/265892 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/authors_and_languages. ___ 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-653308 into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-653308 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #653308 in widelands: "The attack dialog is not updating the number of
possible attackers"
https://bugs.launchpad.net/widelands/+bug/653308
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-653308/+merge/265931
The attack box now updates itself when more / less soldiers are available.
Sliders now update the cursor at once when the min/max value is changed.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-653308 into lp:widelands.
=== modified file 'src/ui_basic/slider.cc'
--- src/ui_basic/slider.cc 2014-11-27 12:02:08 +
+++ src/ui_basic/slider.cc 2015-07-27 07:21:22 +
@@ -113,8 +113,10 @@
*/
void Slider::set_max_value(int32_t new_max) {
assert(m_min_value <= new_max);
- if (m_max_value != new_max)
+ if (m_max_value != new_max) {
+ calc_cursor_pos();
update();
+ }
m_max_value = new_max;
set_value(m_value);
}
@@ -126,8 +128,10 @@
*/
void Slider::set_min_value(int32_t new_min) {
assert(m_max_value >= new_min);
- if (m_min_value != new_min)
+ if (m_min_value != new_min) {
+ calc_cursor_pos();
update();
+ }
m_min_value = new_min;
set_value(m_value);
}
=== modified file 'src/wui/attack_box.cc'
--- src/wui/attack_box.cc 2015-03-30 08:45:09 +
+++ src/wui/attack_box.cc 2015-07-27 07:21:22 +
@@ -30,6 +30,8 @@
#include "graphic/text_constants.h"
#include "logic/soldier.h"
+constexpr int32_t kUpdateTime = 1000; // 1 second, gametime
+
AttackBox::AttackBox
(UI::Panel * parent,
Widelands::Player * player,
@@ -44,7 +46,9 @@
m_node(target),
m_slider_soldiers(nullptr),
m_text_soldiers(nullptr),
- m_add_soldiers(nullptr)
+ m_less_soldiers(nullptr),
+ m_add_soldiers(nullptr),
+ lastupdate_(0)
{
init();
}
@@ -52,6 +56,7 @@
AttackBox::~AttackBox() {
delete m_slider_soldiers;
delete m_text_soldiers;
+ delete m_less_soldiers;
delete m_add_soldiers;
}
@@ -119,6 +124,17 @@
return *button;
}
+/*
+ * Update available soldiers
+ */
+void AttackBox::think() {
+ int32_t const gametime = m_pl->egbase().get_gametime();
+ if ((gametime - lastupdate_) > kUpdateTime) {
+ update_attack();
+ lastupdate_ = gametime;
+ }
+}
+
void AttackBox::update_attack() {
assert(m_slider_soldiers);
assert(m_text_soldiers);
@@ -127,8 +143,9 @@
int32_t max_attackers = get_max_attackers();
- if (m_slider_soldiers->get_max_value() != max_attackers)
+ if (m_slider_soldiers->get_max_value() != max_attackers) {
m_slider_soldiers->set_max_value(max_attackers);
+ }
m_slider_soldiers->set_enabled(max_attackers > 0);
m_add_soldiers->set_enabled(max_attackers > m_slider_soldiers->get_value());
=== modified file 'src/wui/attack_box.h'
--- src/wui/attack_box.h 2015-03-26 18:45:52 +
+++ src/wui/attack_box.h 2015-07-27 07:21:22 +
@@ -76,6 +76,7 @@
void (AttackBox::*fn)(),
const std::string & tooltip_text);
+ void think() override;
void update_attack();
void send_less_soldiers();
void send_more_soldiers();
@@ -90,6 +91,9 @@
UI::Button * m_less_soldiers;
UI::Button * m_add_soldiers;
+
+ /// The last time the information in this Panel got updated
+ uint32_t lastupdate_;
};
#endif // end of include guard: WL_WUI_ATTACK_BOX_H
___
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
