Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1581828 into lp:widelands

[Klaus Halfmann]

Review: Approve compile, code review, test

Loogs good for me, played the originl desync now for > 30 minutes on two 
network computers.
Will try to do more testing tomorrow
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1581828/+merge/297668
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1581828.

___
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/beautiful_correct_lines into lp:widelands

[Klaus Halfmann]

Review: Approve testing

Revied the code and plyed for quit a while,
all fine for me
-- 
https://code.launchpad.net/~widelands-dev/widelands/beautiful_correct_lines/+merge/284517
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/beautiful_correct_lines.

___
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-1395278-base-economy into lp:widelands

[Klaus Halfmann]

Review: Approve code / compile

Compiles for me,
Manual Review in launchapd was fine,
will play on this branch for some time now.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-base-economy/+merge/285289
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1395278-base-economy.

___
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/beautiful_correct_lines into lp:widelands

[Klaus Halfmann]

Hello SirVer:

Id like to move the 0.5 * line_width invariant in tesselate_line_strip out of 
the loop.
(I tend to optimize all the code I see)

--
Hasi50


> Am 05.02.2016 um 20:19 schrieb SirVer :
> 
> If you want to play around a little bit more, go for it. I will not code 
> anymore tonight, so I will only clean this up tomorrow anyways. There are the 
> parameters you can play with
> 
> - In the last commit I removed the code that draws the circles to connect to 
> lines, so lines are not connected anymore. You might bring that back.
> - the fuzzyness of the lines is defined through this line:
>  float alpha = pow(cos(var_color.a * PI / 2), 1.5);
> the corners of each line quad are alpha = 1, opposite are -1, so the shader 
> gets a linear interpolated value between -1 and 1 across the line. This 
> function turns that into a non linear function that defines the falloff. You 
> can replace that or tweak the 1.5 to different values you like.
> - glDepthFunction(GL_LEQUAL) vs glDepthFunction(GL_LESS) defines overdraw 
> behavior for overlapping lines. If two lines overlap, it defines if the 
> second lines pixel are drawn over the first line pixels or not - that 
> obviously only makes a difference if the pixel is partially transparent. 
> - The thickness of the lines in the plot area. 
> 
> It might help to bring back this line in graphic:
> 
> screen_->draw_line_strip(
>   {FloatPoint(20, 300), FloatPoint(700, 500), FloatPoint(250, 
> 50)}, RGBColor(255, 0, 0), 30, LineDrawMode::kAntialiased);
> 
> directly before:
>   RenderQueue::instance().draw(screen_->width(), screen_->height());
> 
> It draws a big fat line across your screen that is much bigger than any we 
> use in Widelands, but it gives you a feeling for your parameters. 
> 
> It is fun to play around with that, but I did it too much now. Maybe you find 
> better parameters. 
> -- 
> https://code.launchpad.net/~widelands-dev/widelands/beautiful_correct_lines/+merge/284517
> Your team Widelands Developers is subscribed to branch 
> lp:~widelands-dev/widelands/beautiful_correct_lines.
> 
> ___
> 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


-- 
https://code.launchpad.net/~widelands-dev/widelands/beautiful_correct_lines/+merge/284517
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/beautiful_correct_lines.

___
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/beautiful_correct_lines into lp:widelands

[Klaus Halfmann]

WL crashed on me after I tried to open a games and iternet game I save some 
versions ago,
Ill try a new game tomorrow unless this should work?
-- 
https://code.launchpad.net/~widelands-dev/widelands/beautiful_correct_lines/+merge/284517
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/beautiful_correct_lines.

___
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-1395278-scripting into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug-1395278-scripting into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-scripting/+merge/285909

* Switch from m_ to _ mostly in scripting
* Fixed a doc warning for clang in songset.h

I expanded some variables to more expressive names
like m_plr -> player_number_

As lua uses m_ for functions as well the code
should be more readable as well.
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-scripting into lp:widelands.
=== modified file 'src/scripting/logic.cc'
--- src/scripting/logic.cc	2015-01-31 16:03:59 +
+++ src/scripting/logic.cc	2016-02-12 17:03:35 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2015 by the Widelands Development Team
+ * Copyright (C) 2006-2016 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -63,22 +63,22 @@
 }  // namespace
 
 LuaEditorInterface::LuaEditorInterface(Widelands::EditorGameBase* g)
-	: m_factory(new EditorFactory())
+	: factory_(new EditorFactory())
 {
-	setup_for_editor_and_game(m_L, g);
-	LuaRoot::luaopen_wlroot(m_L, true);
-	LuaEditor::luaopen_wleditor(m_L);
+	setup_for_editor_and_game(lua_state_, g);
+	LuaRoot::luaopen_wlroot(lua_state_, true);
+	LuaEditor::luaopen_wleditor(lua_state_);
 
 	// Push the factory class into the registry
-	lua_pushlightuserdata(m_L, reinterpret_cast<void*>(dynamic_cast<Factory*>(m_factory.get(;
-	lua_setfield(m_L, LUA_REGISTRYINDEX, "factory");
+	lua_pushlightuserdata(lua_state_, reinterpret_cast<void*>(dynamic_cast<Factory*>(factory_.get(;
+	lua_setfield(lua_state_, LUA_REGISTRYINDEX, "factory");
 }
 
 LuaEditorInterface::~LuaEditorInterface() {
 }
 
 std::unique_ptr LuaEditorInterface::run_script(const std::string& script) {
-	return run_script_maybe_from_map(m_L, script);
+	return run_script_maybe_from_map(lua_state_, script);
 }
 
 // Special handling of math.random.
@@ -125,27 +125,27 @@
 }
 
 LuaGameInterface::LuaGameInterface(Widelands::Game * g)
-	: m_factory(new GameFactory())
+	: factory_(new GameFactory())
 {
-	setup_for_editor_and_game(m_L, g);
+	setup_for_editor_and_game(lua_state_, g);
 
 	// Overwrite math.random
-	lua_getglobal(m_L, "math");
-	lua_pushcfunction(m_L, L_math_random);
-	lua_setfield(m_L, -2, "random");
-	lua_pop(m_L, 1); // pop "math"
+	lua_getglobal(lua_state_, "math");
+	lua_pushcfunction(lua_state_, L_math_random);
+	lua_setfield(lua_state_, -2, "random");
+	lua_pop(lua_state_, 1); // pop "math"
 
-	LuaRoot::luaopen_wlroot(m_L, false);
-	LuaGame::luaopen_wlgame(m_L);
+	LuaRoot::luaopen_wlroot(lua_state_, false);
+	LuaGame::luaopen_wlgame(lua_state_);
 
 	// Push the game into the registry
-	lua_pushlightuserdata(m_L, static_cast(g));
-	lua_setfield(m_L, LUA_REGISTRYINDEX, "game");
+	lua_pushlightuserdata(lua_state_, static_cast(g));
+	lua_setfield(lua_state_, LUA_REGISTRYINDEX, "game");
 
 	// Push the factory class into the registry
 	lua_pushlightuserdata
-		(m_L, reinterpret_cast(dynamic_cast(m_factory.get(;
-	lua_setfield(m_L, LUA_REGISTRYINDEX, "factory");
+		(lua_state_, reinterpret_cast(dynamic_cast(factory_.get(;
+	lua_setfield(lua_state_, LUA_REGISTRYINDEX, "factory");
 }
 
 LuaGameInterface::~LuaGameInterface() {
@@ -153,7 +153,7 @@
 
 LuaCoroutine* LuaGameInterface::read_coroutine(FileRead& fr) {
 	LuaCoroutine * rv = new LuaCoroutine(nullptr);
-	rv->read(m_L, fr);
+	rv->read(lua_state_, fr);
 	return rv;
 }
 
@@ -167,77 +167,77 @@
 	 uint32_t size)
 {
 	// Clean out the garbage before loading.
-	lua_gc(m_L, LUA_GCCOLLECT, 0);
+	lua_gc(lua_state_, LUA_GCCOLLECT, 0);
 
-	assert(lua_gettop(m_L) == 0); // S:
-	unpersist_object(m_L, fr, mol, size);
-	assert(lua_gettop(m_L) == 1); // S: unpersisted_object
-	luaL_checktype(m_L, -1, LUA_TTABLE);
+	assert(lua_gettop(lua_state_) == 0); // S:
+	unpersist_object(lua_state_, fr, mol, size);
+	assert(lua_gettop(lua_state_) == 1); // S: unpersisted_object
+	luaL_checktype(lua_state_, -1, LUA_TTABLE);
 
 	// Now, we have to merge all keys from the loaded table
 	// into the global table
-	lua_pushnil(m_L);  // S: table nil
-	while (lua_next(m_L, 1) != 0) {
+	lua_pushnil(lua_state_);  // S: table nil
+	while (lua_next(lua_state_, 1) != 0) {
 		// S: table key value
-		lua_rawgeti(m_L, LUA_REGISTRYINDEX, LUA_RIDX_GLOBALS);  // S: table key value globals_table
-		lua_pushvalue(m_L, -3); // S: table key value globals_table key
-		lua_gettable(m_L, -2);  // S: table key value globals_table value_in_globals
-		if (lua_compare(m_L, -1, -3, LUA_OPEQ)) {
-			lua_pop(m_L, 3); // S: table key
+		lua_rawgeti

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1395278-scripting into lp:widelands

[Klaus Halfmann]

Hello GunChleoc, I cared for the copyright.

For the type of the player_num / player_index we shoud use the type you 
suggested, of course.
But this is 
a) out of scope of the original bug.
b) I have no Idea what, especially in LUA, might break.-

We should delegate this to some other ticket.

BTW: Who ist the master of LUA in widelands.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-scripting/+merge/285909
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-scripting 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/zip_file_error_messages into lp:widelands

[Klaus Halfmann]

Hmm, clang tell me:

/Users/klaus/develop/widelands-repo/bug-1395278-network/src/io/filesystem/zip_filesystem.cc:549:56:
 warning: format specifies type 'wchar_t *' but the argument has type
  'const value_type *' (aka 'const char *') [-Wformat]
throw wexception("Failed to write into zipfile %S", 
zip_file_->path().c_str());
   ~~   
^
   %s
This may fail in case the File Path conatins a unicode char (like German ä ore 
gaelic à).
how could I force this issue?

-- 
https://code.launchpad.net/~widelands-dev/widelands/zip_file_error_messages/+merge/285711
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/zip_file_error_messages.

___
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:~klaus-halfmann/widelands/bug-1395278-ui_fsmenu into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~klaus-halfmann/widelands/bug-1395278-ui_fsmenu into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~klaus-halfmann/widelands/bug-1395278-ui_fsmenu/+merge/284339

Migration from m_... to ..._ mostly in ui_fsmenu
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~klaus-halfmann/widelands/bug-1395278-ui_fsmenu into lp:widelands.
=== modified file 'src/ui_fsmenu/base.h'
--- src/ui_fsmenu/base.h	2015-12-05 12:51:16 +
+++ src/ui_fsmenu/base.h	2016-01-28 20:02:32 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002, 2006, 2008-2009 by the Widelands Development Team
+ * Copyright (C) 2002-2016 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License

=== modified file 'src/ui_fsmenu/campaign_select.cc'
--- src/ui_fsmenu/campaign_select.cc	2015-10-23 18:03:33 +
+++ src/ui_fsmenu/campaign_select.cc	2016-01-28 20:02:32 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2012 by the Widelands Development Team
+ * Copyright (C) 2002-2016 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -42,71 +42,71 @@
  */
 FullscreenMenuCampaignSelect::FullscreenMenuCampaignSelect() :
 	FullscreenMenuLoadMapOrGame(),
-	m_table(this, tablex_, tabley_, tablew_, tableh_, false),
+	table_(this, tablex_, tabley_, tablew_, tableh_, false),
 
 	// Main Title
-	m_title
+	title_
 		(this, get_w() / 2, tabley_ / 3,
 		 _("Choose a campaign"),
 		 UI::Align_HCenter),
 
 	// Campaign description
-	m_label_campname
+	label_campname_
 		(this, right_column_x_, tabley_,
 		 "",
 		 UI::Align_Left),
-	m_ta_campname(this,
-	  right_column_x_ + indent_, get_y_from_preceding(m_label_campname) + padding_,
-	  get_right_column_w(right_column_x_) - indent_, m_label_height),
-
-	m_label_tribename
-		(this, right_column_x_, get_y_from_preceding(m_ta_campname) + 2 * padding_,
-		 "",
-		 UI::Align_Left),
-	m_ta_tribename(this,
-		 right_column_x_ + indent_, get_y_from_preceding(m_label_tribename) + padding_,
-		 get_right_column_w(right_column_x_ + indent_), m_label_height),
-
-	m_label_difficulty
-		(this, right_column_x_, get_y_from_preceding(m_ta_tribename) + 2 * padding_,
-		 "",
-		 UI::Align_Left),
-	m_ta_difficulty(this,
-		 right_column_x_ + indent_, get_y_from_preceding(m_label_difficulty) + padding_,
-		 get_right_column_w(right_column_x_ + indent_), 2 * m_label_height - padding_),
-
-	m_label_description
-		(this, right_column_x_, get_y_from_preceding(m_ta_difficulty) + 2 * padding_,
+	ta_campname_(this,
+	  right_column_x_ + indent_, get_y_fropreceding_(label_campname_) + padding_,
+	  get_right_column_w(right_column_x_) - indent_, label_height_),
+
+	label_tribename_
+		(this, right_column_x_, get_y_fropreceding_(ta_campname_) + 2 * padding_,
+		 "",
+		 UI::Align_Left),
+	ta_tribename_(this,
+		 right_column_x_ + indent_, get_y_fropreceding_(label_tribename_) + padding_,
+		 get_right_column_w(right_column_x_ + indent_), label_height_),
+
+	label_difficulty_
+		(this, right_column_x_, get_y_fropreceding_(ta_tribename_) + 2 * padding_,
+		 "",
+		 UI::Align_Left),
+	ta_difficulty_(this,
+		 right_column_x_ + indent_, get_y_fropreceding_(label_difficulty_) + padding_,
+		 get_right_column_w(right_column_x_ + indent_), 2 * label_height_ - padding_),
+
+	label_description_
+		(this, right_column_x_, get_y_fropreceding_(ta_difficulty_) + 2 * padding_,
 		 _("Description:"),
 		 UI::Align_Left),
-	m_ta_description
+	ta_description_
 		(this,
 		 right_column_x_ + indent_,
-		 get_y_from_preceding(m_label_description) + padding_,
+		 get_y_fropreceding_(label_description_) + padding_,
 		 get_right_column_w(right_column_x_ + indent_),
-		 m_buty - get_y_from_preceding(m_label_description) - 4 * padding_)
+		 buty_ - get_y_fropreceding_(label_description_) - 4 * padding_)
 {
-	m_title.set_textstyle(UI::TextStyle::ui_big());
+	title_.set_textstyle(UI::TextStyle::ui_big());
 	back_.set_tooltip(_("Return to the main menu"));
 	ok_.set_tooltip(_("Play this campaign"));
-	m_ta_campname.set_tooltip(_("The name of this campaign"));
-	m_ta_tribename.set_tooltip(_("The tribe you will be playing"));
-	m_ta_difficulty.set_tooltip(_("The difficulty of this campaign"));
+	ta_campname_.set_tooltip(_("The name of this campaign"));
+	ta_tribename_.set_tooltip(_("The tribe you will be playing"));
+	ta_difficulty_.set_tooltip(_("The difficulty of this campaign"));
 
 	ok_.sigclicked.connect(boost::bind(::clicked_ok, boost::ref(*this)));
 	back_.sigclicked.connect(boost::bind(::clicked_back, boost

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1395278-logic1 into lp:widelands

[Klaus Halfmann]

Review: Approve

I playes some Atlanters in "the last" bastion without any Hickups,
execpt that the trees so not grow that good in comparioson to r18?

Lets merge this...
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-logic1/+merge/286178
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1395278-logic1.

___
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/network-memory into lp:widelands

[Klaus Halfmann]

Review: Approve compile / test

Compiles, did some test with Gun, which foound bug #1542821 but this is the 
same in trunk.
Maybe some lists will stay in some other state now, but as of the networking 
error there
is no consistent state anyway.

OTOH we get rid of some memory leaks.


Diff comments:

> 
> === modified file 'src/ui_fsmenu/internet_lobby.cc'
> --- src/ui_fsmenu/internet_lobby.cc   2016-02-07 16:31:06 +
> +++ src/ui_fsmenu/internet_lobby.cc   2016-02-19 20:11:01 +
> @@ -176,16 +176,18 @@
>   InternetGaming::ref().handle_metaserver_communication();
>   }
>  
> - if (InternetGaming::ref().update_for_clients())
> + if (InternetGaming::ref().update_for_clients()) {
>   fill_client_list(InternetGaming::ref().clients());
> + }
>  

This is a bad pattern for multithreaded Programms:
if (someLengthyExpression->getSomeThingThatMayBeNull()) {
   // Return value may have changed in some other thread
   someLengthyExpression->getSomeThingThatMayBeNull()->doSomething();
}
Better:
SomeThingThatMayBeNull mayBe = 
someLengthyExpression->getSomeThingThatMayBeNull();
if (mayBe) {
   mayBe->doSomething();
}

C++ may take over new Operators from languangs like swift: 
someLengthyExpression?->getSomeThingThatMayBeNull()?->doSomething()
> https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/OptionalChaining.html#//apple_ref/doc/uid/TP40014097-CH21-ID245

BUT, for Widelands this OK :-)

> - if (InternetGaming::ref().update_for_games())
> + if (InternetGaming::ref().update_for_games()) {
>   fill_games_list(InternetGaming::ref().games());
> + }
>  }
>  
>  void FullscreenMenuInternetLobby::clicked_ok()
>  {
> - if (joingame.enabled()) {
> + if (joingame_.enabled()) {
>   server_doubleclicked();
>   } else {
>   clicked_hostgame();


-- 
https://code.launchpad.net/~widelands-dev/widelands/network-memory/+merge/286162
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/network-memory.

___
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-1395278-logic1 into lp:widelands

[Klaus Halfmann]

Review: Approve compile / code read

See only harmless renamings here,
will play some games now, but this will take some time


Diff comments:

> 
> === modified file 'src/logic/game.cc'
> --- src/logic/game.cc 2016-02-07 07:16:24 +
> +++ src/logic/game.cc 2016-02-16 14:25:27 +
> @@ -1024,40 +1024,40 @@
>   // Read general statistics
>   uint32_t entries = fr.unsigned_16();
>   const PlayerNumber nr_players = map().get_nrplayers();
> - m_general_stats.resize(nr_players);
> + general_stats_.resize(nr_players);
>  
>   iterate_players_existing_novar(p, nr_players, *this) {
> - m_general_stats[p - 1].land_size   .resize(entries);
> - m_general_stats[p - 1].nr_workers  .resize(entries);
> - m_general_stats[p - 1].nr_buildings.resize(entries);
> - m_general_stats[p - 1].nr_wares.resize(entries);
> - m_general_stats[p - 1].productivity.resize(entries);
> - m_general_stats[p - 1].nr_casualties   .resize(entries);
> - m_general_stats[p - 1].nr_kills.resize(entries);
> - m_general_stats[p - 1].nr_msites_lost.resize(entries);
> - m_general_stats[p - 1].nr_msites_defeated.resize(entries);
> - m_general_stats[p - 1].nr_civil_blds_lost.resize(entries);
> - m_general_stats[p - 1].nr_civil_blds_defeated.resize(entries);
> - m_general_stats[p - 1].miltary_strength.resize(entries);
> - m_general_stats[p - 1].custom_statistic.resize(entries);
> + general_stats_[p - 1].land_size   .resize(entries);
> + general_stats_[p - 1].nr_workers  .resize(entries);
> + general_stats_[p - 1].nr_buildings.resize(entries);
> + general_stats_[p - 1].nr_wares.resize(entries);
> + general_stats_[p - 1].productivity.resize(entries);
> + general_stats_[p - 1].nr_casualties   .resize(entries);
> + general_stats_[p - 1].nr_kills.resize(entries);
> + general_stats_[p - 1].nr_msites_lost.resize(entries);
> + general_stats_[p - 1].nr_msites_defeated.resize(entries);
> + general_stats_[p - 1].nr_civil_blds_lost.resize(entries);
> + general_stats_[p - 1].nr_civil_blds_defeated.resize(entries);
> + general_stats_[p - 1].miltary_strength.resize(entries);
> + general_stats_[p - 1].custom_statistic.resize(entries);

We should try to kill such invariants, even if the compiler may find this ...

>   }
>  
>   iterate_players_existing_novar(p, nr_players, *this)
> - for (uint32_t j = 0; j < m_general_stats[p - 
> 1].land_size.size(); ++j)
> + for (uint32_t j = 0; j < general_stats_[p - 
> 1].land_size.size(); ++j)
>   {
> - m_general_stats[p - 1].land_size   [j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_workers  [j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_buildings[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_wares[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].productivity[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_casualties   [j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_kills[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_msites_lost[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_msites_defeated[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_civil_blds_lost[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].nr_civil_blds_defeated[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].miltary_strength[j] = 
> fr.unsigned_32();
> - m_general_stats[p - 1].custom_statistic[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].land_size   [j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_workers  [j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_buildings[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_wares[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].productivity[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_casualties   [j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_kills[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_msites_lost[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_msites_defeated[j] = 
> fr.unsigned_32();
> + general_stats_[p - 1].nr_civil_blds_lost[j] = 
> fr.unsigned_32();
> 

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/network-memory into lp:widelands

[Klaus Halfmann]

Pulling the network cable and then trying to go along the internet game gave me:

InternetGaming: reached a timeout for an awaited answer of the metaserver!
InternetGaming: Connecting to the metaserver.

Warning: Verbindungsproblem
Widelands konnte sich nicht zum Metaserver verbinden.
Assertion failed: (state_ == OFFLINE), function login, file 
/Users/klaus/develop/widelands-repo/network-memory/src/network/internet_gaming.cc,
 line 139.

The "Solution" in Java is returning an immutable vector.
No idea how to do this in C++ perhpas some const& const, whatever...




-- 
https://code.launchpad.net/~widelands-dev/widelands/network-memory/+merge/286162
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/network-memory 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:~klaus-halfmann/widelands/feature-NoClangWarn into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~klaus-halfmann/widelands/feature-NoClangWarn into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~klaus-halfmann/widelands/feature-NoClangWarn/+merge/283521

Added -Wno-documentation for clang to suppress a lot of warnings
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~klaus-halfmann/widelands/feature-NoClangWarn into lp:widelands.
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2016-01-11 20:25:48 +
+++ CMakeLists.txt	2016-01-21 17:52:55 +
@@ -121,6 +121,7 @@
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-unreachable-code")
 
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Werror=non-pod-varargs")
+  wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-documentation")
 
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Qunused-arguments")
 else()

___
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/network-memory into lp:widelands

[Klaus Halfmann]

Mhh, you canged the semantics: 
* now: on Error you do not update anything.
* old: on Error list whre made empty (to indicate the error)

I will play a round with trunk to find how it looks like and then compare wiht 
this branch.
-- 
https://code.launchpad.net/~widelands-dev/widelands/network-memory/+merge/286162
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/network-memory 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-1395278-network into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug-1395278-network into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-network/+merge/285990

Make member variables like _ mostly in network.
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-network into lp:widelands.
=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2016-02-06 11:11:24 +
+++ src/network/internet_gaming.cc	2016-02-14 15:52:10 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2006, 2008-2009, 2012-2013 by the Widelands Development Team
+ * Copyright (C) 2004-2016 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -35,56 +35,56 @@
 /// Private constructor by purpose: NEVER call directly. Always call InternetGaming::ref(), this will ensure
 /// that only one instance is running at time.
 InternetGaming::InternetGaming() :
-	m_sock   (nullptr),
-	m_sockset(nullptr),
-	m_state  (OFFLINE),
-	m_reg(false),
-	m_port   (INTERNET_GAMING_PORT),
-	m_clientrights   (INTERNET_CLIENT_UNREGISTERED),
-	m_gameip (""),
-	clientupdateonmetaserver (true),
-	gameupdateonmetaserver   (true),
-	clientupdate (false),
-	gameupdate   (false),
-	time_offset  (0),
-	waittimeout  (std::numeric_limits::max()),
-	lastping (time(nullptr))
+	sock_(nullptr),
+	sockset_ (nullptr),
+	state_   (OFFLINE),
+	reg_ (false),
+	port_(INTERNET_GAMING_PORT),
+	clientrights_(INTERNET_CLIENT_UNREGISTERED),
+	gameip_  (""),
+	clientupdateonmetaserver_(true),
+	gameupdateonmetaserver_  (true),
+	clientupdate_(false),
+	gameupdate_  (false),
+	time_offset_ (0),
+	waittimeout_ (std::numeric_limits::max()),
+	lastping_(time(nullptr))
 {
 	// Fill the list of possible messages from the server
 	InternetGamingMessages::fill_map();
 
 	// Set connection tracking variables to 0
-	lastbrokensocket[0]  = 0;
-	lastbrokensocket[1]  = 0;
+	lastbrokensocket_[0]  = 0;
+	lastbrokensocket_[1]  = 0;
 
 }
 
 /// resets all stored variables without the chat messages for a clean new login (not relogin)
 void InternetGaming::reset() {
-	m_sock   = nullptr;
-	m_sockset= nullptr;
-	m_state  = OFFLINE;
-	m_pwd= "";
-	m_reg= false;
-	m_meta   = INTERNET_GAMING_METASERVER;
-	m_port   = INTERNET_GAMING_PORT;
-	m_clientname = "";
-	m_clientrights   = INTERNET_CLIENT_UNREGISTERED;
-	m_gamename   = "";
-	m_gameip = "";
-	clientupdateonmetaserver = true;
-	gameupdateonmetaserver   = true;
-	clientupdate = false;
-	gameupdate   = false;
-	time_offset  = 0;
-	waitcmd  = "";
-	waittimeout  = std::numeric_limits::max();
-	lastbrokensocket[0]  = 0;
-	lastbrokensocket[1]  = 0;
-	lastping = time(nullptr);
+	sock_   = nullptr;
+	sockset_= nullptr;
+	state_  = OFFLINE;
+	pwd_= "";
+	reg_= false;
+	meta_   = INTERNET_GAMING_METASERVER;
+	port_   = INTERNET_GAMING_PORT;
+	clientname_ = "";
+	clientrights_   = INTERNET_CLIENT_UNREGISTERED;
+	gamename_   = "";
+	gameip_ = "";
+	clientupdateonmetaserver_= true;
+	gameupdateonmetaserver_  = true;
+	clientupdate_= false;
+	gameupdate_  = false;
+	time_offset_ = 0;
+	waitcmd_ = "";
+	waittimeout_ = std::numeric_limits::max();
+	lastbrokensocket_[0] = 0;
+	lastbrokensocket_[1] = 0;
+	lastping_= time(nullptr);
 
-	clientlist.clear();
-	gamelist.clear();
+	clientlist_.clear();
+	gamelist_.clear();
 }
 
 
@@ -104,18 +104,18 @@
 	// First of all try to connect to the metaserver
 	log("InternetGaming: Connecting to the metaserver.\n");
 	IPaddress peer;
-	if (hostent * const he = gethostbyname(m_meta.c_str())) {
+	if (hostent * const he = gethostbyname(meta_.c_str())) {
 		peer.host = (reinterpret_cast(he->h_addr_list[0]))->s_addr;
 DIAG_OFF("-Wold-style-cast")
-		peer.port = htons(m_port);
+		peer.port = htons(por

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lock_game_logic into lp:widelands

[Klaus Halfmann]

Hello SirVer:

Id propose a small change, mostly comments only

=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc   2016-02-13 19:17:06 +
+++ src/ui_basic/panel.cc   2016-02-14 11:20:39 +
@@ -148,9 +148,10 @@
// Panel-specific startup code. This might call end_modal()!
start();
 
-   // think() is called at most 15 times per second.
+   // think() is called at most 15 times per second, thats every 66ms
const uint32_t kGameLogicDelay = 1000 / 15;
 
+   // with a default of 30 FPS the gemay will be drawn every 33ms
const uint32_t draw_delay =
   1000 / std::max(5, 
g_options.pull_section("global").get_int("maxfps", 30));
 
@@ -163,8 +164,9 @@
Panel::ui_mousewheel
};
 
-   uint32_t next_think_time = SDL_GetTicks() + kGameLogicDelay;
-   uint32_t next_draw_time = SDL_GetTicks() + draw_delay;
+   uint32_t sdl_ticks = SDL_GetTicks();
+   uint32_t next_think_time = sdl_ticks + kGameLogicDelay;
+   uint32_t next_draw_time  = sdl_ticks + draw_delay;
while (_running) {
const uint32_t start_time = SDL_GetTicks();


I would like to test this „in the wild“ for a while.



-- 
https://code.launchpad.net/~widelands-dev/widelands/lock_game_logic/+merge/285980
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/lock_game_logic.

___
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/lock_game_logic into lp:widelands

[Klaus Halfmann]

Hello Kaputtnick:

I just review that code, you wrote:

> Is there something specific to look at?

I think we should do tests in the „real world“ with different FPS seconds and a 
real
Network jitter. SirVers approach feels correct to me, but I am not sure is this
may have adverse effects, as the game logic now will run at 15 thinks()/s 
compared
to 25/s (with default settings) before.

Worst secario would we a slow computer in australia on ad 1024k Modem (Windows)
vs. a Workstation on Linux/OSX on 1 fibre net. But I tend to exaggerate such 
issues :-)


> I tested a short 2Player game against AI and found nothing strange.

This should not affect any local game, AFAI can imagine


> Does this affect calculation of statistics f.e. the productivity shown on 
> each building?

No idea, good question,

> Is a game with 8 players better to test this changes?

Yes as many players as spoonable with different FPS.
But not in a LAN-Party :-)



-- 
https://code.launchpad.net/~widelands-dev/widelands/lock_game_logic/+merge/285980
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/lock_game_logic.

___
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/lock_game_logic into lp:widelands

[Klaus Halfmann]

A test with TinoM palying on Impact with an AI, failed, going to reprodcu this
-- 
https://code.launchpad.net/~widelands-dev/widelands/lock_game_logic/+merge/285980
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/lock_game_logic.

___
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-1395278-logic2 into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug-1395278-logic2 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1395278 in widelands: "Consolidate naming of member variables"
  https://bugs.launchpad.net/widelands/+bug/1395278

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-logic2/+merge/287687

Review is ok for me, pure renaming, nothing else,
will play some open network game now...
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-logic2 into lp:widelands.
=== modified file 'src/game_io/game_player_economies_packet.cc'
--- src/game_io/game_player_economies_packet.cc	2015-11-28 22:29:26 +
+++ src/game_io/game_player_economies_packet.cc	2016-03-01 18:23:35 +
@@ -48,7 +48,7 @@
 		if (packet_version == kCurrentPacketVersion) {
 			iterate_players_existing(p, nr_players, game, player)
 try {
-	Player::Economies & economies = player->m_economies;
+	Player::Economies & economies = player->economies_;
 	for (uint32_t i = 0; i < economies.size(); ++i) {
 		uint32_t value = fr.unsigned_32();
 		if (value < 0x) {
@@ -106,7 +106,7 @@
 	const Field & field_0 = map[0];
 	PlayerNumber const nr_players = map.get_nrplayers();
 	iterate_players_existing_const(p, nr_players, game, player) {
-		const Player::Economies & economies = player->m_economies;
+		const Player::Economies & economies = player->economies_;
 		for (Economy * temp_economy : economies) {
 			bool wrote_this_economy = false;
 

=== modified file 'src/game_io/game_player_info_packet.cc'
--- src/game_io/game_player_info_packet.cc	2016-01-22 19:53:32 +
+++ src/game_io/game_player_info_packet.cc	2016-03-01 18:23:35 +
@@ -64,12 +64,12 @@
 	player.read_statistics(fr);
 	player.read_remaining_shipnames(fr);
 
-	player.m_casualties = fr.unsigned_32();
-	player.m_kills  = fr.unsigned_32();
-	player.m_msites_lost = fr.unsigned_32();
-	player.m_msites_defeated = fr.unsigned_32();
-	player.m_civil_blds_lost = fr.unsigned_32();
-	player.m_civil_blds_defeated = fr.unsigned_32();
+	player.casualties_ = fr.unsigned_32();
+	player.kills_  = fr.unsigned_32();
+	player.msites_lost_ = fr.unsigned_32();
+	player.msites_defeated_ = fr.unsigned_32();
+	player.civil_blds_lost_ = fr.unsigned_32();
+	player.civil_blds_defeated_ = fr.unsigned_32();
 }
 			}
 			game.read_statistics(fr);
@@ -95,9 +95,9 @@
 	iterate_players_existing_const(p, nr_players, game, plr) {
 		fw.unsigned_8(1); // Player is in game.
 
-		fw.unsigned_8(plr->m_see_all);
+		fw.unsigned_8(plr->see_all_);
 
-		fw.unsigned_8(plr->m_plnum);
+		fw.unsigned_8(plr->player_number_);
 		fw.unsigned_8(plr->team_number());
 
 		fw.c_string(plr->tribe().name().c_str());
@@ -107,8 +107,8 @@
 
 		// Economies are in a packet after map loading
 
-		fw.c_string(plr->m_name.c_str());
-		fw.c_string(plr->m_ai.c_str());
+		fw.c_string(plr->name_.c_str());
+		fw.c_string(plr->ai_.c_str());
 
 		plr->write_statistics(fw);
 		plr->write_remaining_shipnames(fw);

=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc	2016-02-08 20:44:17 +
+++ src/logic/editor_game_base.cc	2016-03-01 18:23:35 +
@@ -184,7 +184,7 @@
 	(MapIndex const i, PlayerNumber const new_owner)
 {
 	iterate_players_existing_const(plnum, MAX_PLAYERS, *this, p) {
-		Player::Field & player_field = p->m_fields[i];
+		Player::Field & player_field = p->fields_[i];
 		if (1 < player_field.vision) {
 			player_field.owner = new_owner;
 		}
@@ -195,7 +195,7 @@
 {
 	if (!Road::is_road_descr(descr))
 		iterate_players_existing_const(plnum, MAX_PLAYERS, *this, p) {
-			Player::Field & player_field = p->m_fields[i];
+			Player::Field & player_field = p->fields_[i];
 			if (1 < player_field.vision) {
 player_field.map_object_descr[TCoords<>::None] = descr;
 			}
@@ -523,7 +523,7 @@
 	MapIndex const   i = f.field - _field;
 	MapIndex const neighbour_i = neighbour.field - _field;
 	iterate_players_existing_const(plnum, MAX_PLAYERS, *this, p) {
-		Player::Field & first_player_field = *p->m_fields;
+		Player::Field & first_player_field = *p->fields_;
 		Player::Field & player_field = (_player_field)[i];
 		if
 			(1 < player_field  .vision

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2016-02-22 08:50:04 +
+++ src/logic/map.cc	2016-03-01 18:23:35 +
@@ -1694,12 +1694,12 @@
 	start = FCoords(instart, [](instart));
 	end   = FCoords(inend,   [](inend));
 
-	path.m_path.clear();
+	path.path_.clear();
 
 	// Some stupid cases...
 	if (start == end) {
-		path.m_start = start;
-		path.m_end = end;
+		path.start_ = start;

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1395278-logic2 into lp:widelands

[Klaus Halfmann]

Played this for seom 40 Minutes now, had no Problems whatsoever
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-logic2/+merge/287687
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-logic2 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/bug-1395278-map_io into lp:widelands

[Klaus Halfmann]

Review: Approve

Found only renamings, execpt for one improvement,
fine for me. 

Will now play some Artifact hunting for completeness...
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-map_io/+merge/288859
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1395278-map_io.

___
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-1191556-cancel-expedition into lp:widelands

[Klaus Halfmann]

The crash happens when you return to the internetlobby, when the computer was 
offline meanwhile.
Looks like on of the Nullpointers in the Netowk code hit us. Still it did not 
happen yesterday.
I may have to provoke it a bit harder, perhaps.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1191556-cancel-expedition/+merge/288375
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1191556-cancel-expedition.

___
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-1191556-cancel-expedition into lp:widelands

[Klaus Halfmann]

Review: Approve

Code looks good, still want to play a bit with the Ports on
my https://wl.widelands.org/maps/fjord-ilands2/ map.

And some refactoring toward clean code is always good :-)
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1191556-cancel-expedition/+merge/288375
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1191556-cancel-expedition.

___
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/dismantlesite-color into lp:widelands

[Klaus Halfmann]

See my inline comment.

Diff comments:

> === modified file 'src/logic/map_objects/tribes/dismantlesite.cc'
> --- src/logic/map_objects/tribes/dismantlesite.cc 2016-02-18 18:27:52 
> +
> +++ src/logic/map_objects/tribes/dismantlesite.cc 2016-03-13 09:28:07 
> +
> @@ -93,7 +94,8 @@
>  void DismantleSite::update_statistics_string(std::string* s)
>  {
>   unsigned int percent = (get_built_per64k() * 100) >> 16;
> - *s = (boost::format(_("%u%% dismantled")) % percent).str();
> + *s = (boost::format("%s") % 
> UI_FONT_CLR_DARK.hex_value() %
> + (boost::format(_("%u%% dismantled")) % 
> percent).str()).str();

Uhm, should this not be like:
boost::format("_(%u%% dismantled)")) % 
UI_FONT_CLR_DARK.hex_value() % percent").str();

Or will this clash with the translation of _(...)?
Im not not sure about this boost::format but this ".str()).str()" is strange 
duplicate?

As I am kind of colorblind I will not comment on the color actally produced :-)

>  }
>  
>  /*


-- 
https://code.launchpad.net/~widelands-dev/widelands/dismantlesite-color/+merge/288866
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/dismantlesite-color 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/bug-1191556-cancel-expedition into lp:widelands

[Klaus Halfmann]

Review: Approve

Played this with a lot of Ports and about > 6 Expeditions for some 3 hours.
I found a Problem with the network lobby (crash if Computer went offline
during the game) but that cannot be related to this code. 

Compiled it again, but found it is alreday in trunk.

I will stick to reviews and testing as long as there are so many branches
to check and I do not have that much time.




-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1191556-cancel-expedition/+merge/288375
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1191556-cancel-expedition.

___
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/python3 into lp:widelands

[Klaus Halfmann]

OK, thas as far as I came, too but now I get
$ ./regression_test.py -b ./widelands 
...
File "./regression_test.py", line 103, in run_widelands
  stdout_file.write(line)
TypeError: must be str, not bytes

I used 
   print(line, flush=True)
for
   stdout_file.write(line)
   stdout_file.flush()

but this in not python2.7 compatible :-(

In addition I used 

< "Widelands exited abnormally. %s" % common_msg
---
> "Widelands exited abnormally. " + common_msg

< "Not all tests pass. %s." % common_msg
---
> "Not all tests pass. " + common_msg

but perhaos that .format() aproach is more compatible
-- 
https://code.launchpad.net/~widelands-dev/widelands/python3/+merge/291236
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/python3 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/python3 into lp:widelands

[Klaus Halfmann]

Looks like you know more about python than I do,
lets check this ...
-- 
https://code.launchpad.net/~widelands-dev/widelands/python3/+merge/291236
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/python3 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/python3 into lp:widelands

[Klaus Halfmann]

Mhh, even using python 2.7 does not work, it just gets stuck in the splasg 
screeen?
Was there some change in the Lua Binding?
-- 
https://code.launchpad.net/~widelands-dev/widelands/python3/+merge/291236
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/python3 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/rename_editor_files into lp:widelands

[Klaus Halfmann]

Review: Approve

OK, found only the expected renamings.
OK, Compiles on OSX 

I have seen no actual code changes -> Approve
-- 
https://code.launchpad.net/~widelands-dev/widelands/rename_editor_files/+merge/291093
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/rename_editor_files.

___
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/rename_editor_files into lp:widelands

[Klaus Halfmann]

Ah, here is the assertion:

WARNING: Player 1 has no starting position - illegal coordinates (29508, 22202).
WARNING: Player 2 has no starting position - illegal coordinates (-1, -1).
WARNING: Player 1 has no starting position - illegal coordinates (29508, 22202).
WARNING: Player 2 has no starting position - illegal coordinates (-1, -1).
WARNING: Player 3 has no starting position - illegal coordinates (-1, -1).
Assertion failed: (1 <= p && p <= get_nrplayers()), function get_starting_pos, 
file /Users/klaus/develop/widelands-repo/rename_editor_files/src/logic/map.h, 
line 205.
-- 
https://code.launchpad.net/~widelands-dev/widelands/rename_editor_files/+merge/291093
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/rename_editor_files.

___
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/rename_editor_files into lp:widelands

[Klaus Halfmann]

perhaps to fast: /

Got a Crash in creating a radom Map 80x80 for 3 players in
Widelands::Map::get_starting_pos(unsigned char) const + 130 (map.h:205)
editor_tool_set_starting_pos_callback(Widelands::TCoords 
const&, 
-- 
https://code.launchpad.net/~widelands-dev/widelands/rename_editor_files/+merge/291093
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/rename_editor_files.

___
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

[Klaus Halfmann]

Review: Needs Fixing regression test

OK, here is the Backtrace,
I think Appvoyer found something similar.

Widelands::EditorGameBase::create_immovable(Widelands::Coords, unsigned char, 
Widelands::MapObjectDescr::OwnerType) + 160 (editor_game_base.cc:360)
Widelands::Worker::run_geologist_find(Widelands::Game&, Widelands::Bob::State&, 
Widelands::Worker::Action const&) + 493 (worker.cc:917)
Widelands::Worker::program_update(Widelands::Game&, Widelands::Bob::State&) + 
1119 (worker.cc:1907)

-- 
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

[Klaus Halfmann]

Review: Needs Information

* 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.

* OK, I cannot use an old replay, as of UnhandledVersionError

* Got a crash in the regression tests, will add info for this later

-- 
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

[Klaus Halfmann]

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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/test-ngettext into lp:widelands

[Klaus Halfmann]

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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/rename_editor_files into lp:widelands

[Klaus Halfmann]

Mhh, looks like _setting_ the playerpos succeed but nrplayers is inconsistant, 
mmh.
Should be unrelated to this branch.
-- 
https://code.launchpad.net/~widelands-dev/widelands/rename_editor_files/+merge/291093
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/rename_editor_files.

___
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

[Klaus Halfmann]

Review: Approve review/compile

Moste of the code is correct, we mave some "Upgrades" form 8bit to 32bit,
that where broken before but got unnoticed, but now they will become visible.
Please check my inline comments

Id really like to have some coverage tool that checks that this code is actually
tested. Still OK for me.

Will run some test now and try to play a game perhpas today.

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;

Values of this calucation may have changed as the cast to uint8_t may have 
limited the value to 256,
I took a closer look but got not clue what this is finally about, mmh.

>   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;

Mhh, instead of protection against underflow, we now dow this we the next 
statement.

>   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)

a const was lost here?

>  {
>   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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/watchwindow-fixes into lp:widelands

[Klaus Halfmann]

I am still missing some comments about the basic
workings of this (very sepcial) window. and I have some questions:

* why is uint8_t as index, a plain unsigned int would not make a difference?
* why is there no visual response when adding the last view fails?


I cannot reproduce #1553699 directly, as the filesaveversion sha changed, uhm:

roaddata: road 455: [...watchwindow-fixes/src/economy/request.cc:172] request: 
[...watchwindow-fixes/src/logic/map_objects/tribes/requirements.cc:51] 
requirements: 

UnhandledVersionError: 

I must tray with some new game.

-- 
https://code.launchpad.net/~widelands-dev/widelands/watchwindow-fixes/+merge/289573
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/watchwindow-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/watchwindow-fixes into lp:widelands

[Klaus Halfmann]

Ill try to reproduce #1553699 whit this perhaps today ...
-- 
https://code.launchpad.net/~widelands-dev/widelands/watchwindow-fixes/+merge/289573
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/watchwindow-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/watchwindow-fixes into lp:widelands

[Klaus Halfmann]

Review: Approve test / compile

Bug #1553699? is fixed with this branch, 
just reproduced this on bzr7903[trunk],
and found it fixed it here in bzr7904[watchwindow-fixes] 

@Miroslav Remák: thanks for fixing this.
-- 
https://code.launchpad.net/~widelands-dev/widelands/watchwindow-fixes/+merge/289573
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/watchwindow-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-1395278-editor into lp:widelands

[Klaus Halfmann]

Played this for a while now, found no anomalies.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-editor/+merge/289494
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1395278-editor.

___
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/revised_maps into lp:widelands

[Klaus Halfmann]

Found a crash after playing archipelago for quite a while,
the game crashed when I quit, but I had not time to check yesterday.
But that should not be related to this branch, I think.
-- 
https://code.launchpad.net/~widelands-dev/widelands/revised_maps/+merge/289704
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/revised_maps.

___
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-1395278-network-io-wui into lp:widelands

[Klaus Halfmann]

Review: Approve

Found only renamings that are fine with me.
Copiled this and played it a while, found no anomalies.

Diff comments:

> === modified file 'src/io/filesystem/layered_filesystem.cc'
> --- src/io/filesystem/layered_filesystem.cc   2016-02-18 18:27:52 +
> +++ src/io/filesystem/layered_filesystem.cc   2016-03-19 10:27:47 +
> @@ -89,7 +89,7 @@
>   results.insert(*fnit);
>   }
>  
> - for (auto it = m_filesystems.rbegin(); it != m_filesystems.rend(); 
> ++it) {
> + for (auto it = filesystems_.rbegin(); it != filesystems_.rend(); ++it) {
>   files = (*it)->list_directory(path);
>  

Is this a candidate for this "auto const" optimizations?

>   for (FilenameSet::iterator fnit = files.begin(); fnit != 
> files.end(); ++fnit)


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-network-io-wui/+merge/289557
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1395278-network-io-wui.

___
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/revised_maps into lp:widelands

[Klaus Halfmann]

Archipelago works for Atlanters, so it should work for pretty every tribe.
Ill try another Map the next days. One of the three AIs 
was able to develop at least a bit. You will need horses (ochsen, donkeys)
in the long term however
-- 
https://code.launchpad.net/~widelands-dev/widelands/revised_maps/+merge/289704
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/revised_maps.

___
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/revised_maps into lp:widelands

[Klaus Halfmann]

Mhh, I found #1562332 again, but this is yet another other Issue, can someone 
elese reproduce this?
-- 
https://code.launchpad.net/~widelands-dev/widelands/revised_maps/+merge/289704
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/revised_maps.

___
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-1395278-logic3 into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug-1395278-logic3 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1395278 in widelands: "Consolidate naming of member variables"
  https://bugs.launchpad.net/widelands/+bug/1395278

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-logic3/+merge/287827

Did a code review, found pure renaming only,
will continue to play the game from the last review (bug-1395278-logic2) now.
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-logic3 into lp:widelands.
=== modified file 'src/game_io/game_cmd_queue_packet.cc'
--- src/game_io/game_cmd_queue_packet.cc	2016-02-15 23:26:42 +
+++ src/game_io/game_cmd_queue_packet.cc	2016-03-02 18:10:45 +
@@ -41,7 +41,7 @@
 		if (packet_version == kCurrentPacketVersion) {
 			CmdQueue & cmdq = game.cmdqueue();
 
-			// nothing to be done for m_game
+			// nothing to be done for game_
 
 			// Next serial
 			cmdq.nextserial_ = fr.unsigned_32();
@@ -86,7 +86,7 @@
 
 	const CmdQueue & cmdq = game.cmdqueue();
 
-	// nothing to be done for m_game
+	// nothing to be done for game_
 
 	// Next serial
 	fw.unsigned_32(cmdq.nextserial_);

=== modified file 'src/game_io/game_loader.cc'
--- src/game_io/game_loader.cc	2016-01-20 20:12:00 +
+++ src/game_io/game_loader.cc	2016-03-02 18:10:45 +
@@ -40,7 +40,7 @@
 namespace Widelands {
 
 GameLoader::GameLoader(const std::string & path, Game & game) :
-	m_fs(*g_fs->make_sub_file_system(path)), m_game(game)
+	m_fs(*g_fs->make_sub_file_system(path)), game_(game)
 {}
 
 
@@ -53,7 +53,7 @@
  */
 int32_t GameLoader::preload_game(GamePreloadPacket & mp) {
 	// Load elemental data block
-	mp.read(m_fs, m_game, nullptr);
+	mp.read(m_fs, game_, nullptr);
 
 	return 0;
 }
@@ -65,43 +65,43 @@
 	ScopedTimer timer("GameLoader::load() took %ums");
 
 	log("Game: Reading Preload Data ... ");
-	{GamePreloadPacket p; p.read(m_fs, m_game);}
+	{GamePreloadPacket p; p.read(m_fs, game_);}
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Game Class Data ... ");
-	{GameClassPacket  p; p.read(m_fs, m_game);}
+	{GameClassPacket  p; p.read(m_fs, game_);}
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Map Data ... ");
-	GameMapPacket M;  M.read(m_fs, m_game);
+	GameMapPacket M;  M.read(m_fs, game_);
 	log("Game: Reading Map Data took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Player Info ... ");
-	{GamePlayerInfoPacket p; p.read(m_fs, m_game);}
+	{GamePlayerInfoPacket p; p.read(m_fs, game_);}
 	log("Game: Reading Player Info took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Calling read_complete()\n");
-	M.read_complete(m_game);
+	M.read_complete(game_);
 	log("Game: read_complete took: %ums\n", timer.ms_since_last_query());
 
 	MapObjectLoader * const mol = M.get_map_object_loader();
 
 	log("Game: Reading Player Economies Info ... ");
-	{GamePlayerEconomiesPacketp; p.read(m_fs, m_game, mol);}
+	{GamePlayerEconomiesPacketp; p.read(m_fs, game_, mol);}
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading ai persistent data ... ");
-	{GamePlayerAiPersistentPacket   p; p.read(m_fs, m_game, mol);}
+	{GamePlayerAiPersistentPacket   p; p.read(m_fs, game_, mol);}
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Command Queue Data ... ");
-	{GameCmdQueuePacket   p; p.read(m_fs, m_game, mol);}
+	{GameCmdQueuePacket   p; p.read(m_fs, game_, mol);}
 	log("took %ums\n", timer.ms_since_last_query());
 
 	//  This must be after the command queue has been read.
 	log("Game: Parsing messages ... ");
-	PlayerNumber const nr_players = m_game.map().get_nrplayers();
-	iterate_players_existing_const(p, nr_players, m_game, player) {
+	PlayerNumber const nr_players = game_.map().get_nrplayers();
+	iterate_players_existing_const(p, nr_players, game_, player) {
 		const MessageQueue & messages = player->messages();
 		for (std::pair<MessageId, Message *> temp_message : messages) {
 			Message* m = temp_message.second;
@@ -109,7 +109,7 @@
 
 			// Renew MapObject connections
 			if (m->serial() > 0) {
-MapObject* mo = m_game.objects().get_object(m->serial());
+MapObject* mo = game_.objects().get_object(m->serial());
 mo->removed.connect
 	(boost::bind(::message_object_removed, player, m_id));
 			}
@@ -118,14 +118,14 @@
 	log("took %ums\n", timer.ms_sinc

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1395278-game_io into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug-1395278-game_io into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1395278 in widelands: "Consolidate naming of member variables"
  https://bugs.launchpad.net/widelands/+bug/1395278

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-game_io/+merge/288172

Checked the code and played a game (with some stores and loads)
LGTM
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-game_io into lp:widelands.
=== modified file 'src/game_io/game_loader.cc'
--- src/game_io/game_loader.cc	2016-03-02 16:30:42 +
+++ src/game_io/game_loader.cc	2016-03-04 20:31:45 +
@@ -40,12 +40,16 @@
 namespace Widelands {
 
 GameLoader::GameLoader(const std::string & path, Game & game) :
+<<<<<<< TREE
 	m_fs(*g_fs->make_sub_file_system(path)), game_(game)
+===
+	fs_(*g_fs->make_sub_file_system(path)), game_(game)
+>>>>>>> MERGE-SOURCE
 {}
 
 
 GameLoader::~GameLoader() {
-	delete _fs;
+	delete _;
 }
 
 /*
@@ -53,7 +57,11 @@
  */
 int32_t GameLoader::preload_game(GamePreloadPacket & mp) {
 	// Load elemental data block
+<<<<<<< TREE
 	mp.read(m_fs, game_, nullptr);
+===
+	mp.read(fs_, game_, nullptr);
+>>>>>>> MERGE-SOURCE
 
 	return 0;
 }
@@ -65,19 +73,35 @@
 	ScopedTimer timer("GameLoader::load() took %ums");
 
 	log("Game: Reading Preload Data ... ");
+<<<<<<< TREE
 	{GamePreloadPacket p; p.read(m_fs, game_);}
+===
+	{GamePreloadPacket p; p.read(fs_, game_);}
+>>>>>>> MERGE-SOURCE
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Game Class Data ... ");
+<<<<<<< TREE
 	{GameClassPacket  p; p.read(m_fs, game_);}
+===
+	{GameClassPacket  p; p.read(fs_, game_);}
+>>>>>>> MERGE-SOURCE
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Map Data ... ");
+<<<<<<< TREE
 	GameMapPacket M;  M.read(m_fs, game_);
+===
+	GameMapPacket M;  M.read(fs_, game_);
+>>>>>>> MERGE-SOURCE
 	log("Game: Reading Map Data took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Player Info ... ");
+<<<<<<< TREE
 	{GamePlayerInfoPacket p; p.read(m_fs, game_);}
+===
+	{GamePlayerInfoPacket p; p.read(fs_, game_);}
+>>>>>>> MERGE-SOURCE
 	log("Game: Reading Player Info took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Calling read_complete()\n");
@@ -87,15 +111,27 @@
 	MapObjectLoader * const mol = M.get_map_object_loader();
 
 	log("Game: Reading Player Economies Info ... ");
+<<<<<<< TREE
 	{GamePlayerEconomiesPacketp; p.read(m_fs, game_, mol);}
+===
+	{GamePlayerEconomiesPacketp; p.read(fs_, game_, mol);}
+>>>>>>> MERGE-SOURCE
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading ai persistent data ... ");
+<<<<<<< TREE
 	{GamePlayerAiPersistentPacket   p; p.read(m_fs, game_, mol);}
+===
+	{GamePlayerAiPersistentPacket   p; p.read(fs_, game_, mol);}
+>>>>>>> MERGE-SOURCE
 	log("took %ums\n", timer.ms_since_last_query());
 
 	log("Game: Reading Command Queue Data ... ");
+<<<<<<< TREE
 	{GameCmdQueuePacket   p; p.read(m_fs, game_, mol);}
+===
+	{GameCmdQueuePacket   p; p.read(fs_, game_, mol);}
+>>>>>>> MERGE-SOURCE
 	log("took %ums\n", timer.ms_since_last_query());
 
 	//  This must be after the command queue has been read.
@@ -104,14 +140,19 @@
 	iterate_players_existing_const(p, nr_players, game_, player) {
 		const MessageQueue & messages = player->messages();
 		for (std::pair<MessageId, Message *> temp_message : messages) {
-			Message* m = temp_message.second;
-			MessageId m_id = temp_message.first;
+			Message* message = temp_message.second;
+			MessageId message_id = temp_message.first;
 
 			// Renew MapObject connections
+<<<<<<< TREE
 			if (m->serial() > 0) {
 MapObject* mo = game_.objects().get_object(m->serial());
+===
+			if (message->serial() > 0) {
+MapObject* mo = game_.objects().get_object(message->serial());
+>>>>>>> MERGE-SOURCE
 mo->removed.connect
-	(boost::bind(::message_object_removed, player, m_id));
+	(boost:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1395278-logic3 into lp:widelands

[Klaus Halfmann]

Played perhas 30 Minutes had no 
issues except an annyoing attacking AI :-)
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1395278-logic3/+merge/287827
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1395278-logic3 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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

Expect a commit in some minutes

Diff comments:

> 
> === modified file 'src/ai/ai_help_structs.h'
> --- src/ai/ai_help_structs.h  2016-03-12 20:06:24 +
> +++ src/ai/ai_help_structs.h  2016-04-02 16:49:56 +
> @@ -351,8 +350,8 @@
>   uint16_t mines_percent;  // % of res it can mine
>   uint32_t current_stats;
>  
> - std::vector inputs;
> - std::vector outputs;
> + std::vector inputs;
> + std::vector outputs;

Yes, will commit some commetns for this one

>   std::vector critical_building_material;
>  
>   bool produces_building_material;
> 
> === modified file 
> 'src/editor/ui_menus/editor_tool_change_resources_options_menu.cc'
> --- src/editor/ui_menus/editor_tool_change_resources_options_menu.cc  
> 2016-03-14 18:10:09 +
> +++ src/editor/ui_menus/editor_tool_change_resources_options_menu.cc  
> 2016-04-02 16:49:56 +
> @@ -197,10 +197,7 @@
>   case Change_By_Increase: change_by += change_by < 63; break;
>   case Change_By_Decrease: change_by -= 1 < change_by;  break;
>   caseSet_To_Increase: set_to+= set_to< 63; break;
> - caseSet_To_Decrease: set_to-= 0 < set_to; break;
> - default:
> - NEVER_HERE();
> - break;
> + caseSet_To_Decrease: set_to-= 0 < set_to;

NOOP - No-operation Mnenomic from Assembler, meaning an Operation that does 
nothing. 
Sometimes needed for technical reasons.
But is thins case we could just write
set_to < set_to; 
which would always be false ?
...
Wait a minute: that is this confusing operator precedence and 
C/C++ not having a real boolean. This actually is:
if (set_to >0) { set_to -= 1; }

So the code is OK, yet confusing.

>   }
>   increase_tool_.set_change_by(change_by);
>   increase_tool_.decrease_tool().set_change_by(change_by);
> 
> === modified file 'src/scripting/lua_globals.cc'
> --- src/scripting/lua_globals.cc  2015-02-28 17:43:48 +
> +++ src/scripting/lua_globals.cc  2016-04-02 16:49:56 +
> @@ -104,7 +104,6 @@
>   case LUA_TTHREAD:
>   case LUA_TLIGHTUSERDATA:
>   report_error(L, "Cannot format the 
> given type %s at index %i", lua_typename(L, i), i);
> - break;

On my way

>  
>   default:
>   {


-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

Review: Approve compile, review

Looks good to me, with Apple clang I get a lot less warnings.
You removed on break; that looks incorrect, but I am but sure,
see diff comments.

I prepared some changes that would remove some implicit float -> double 
warnings, too.
Not sure if I should commit these, though.

Diff comments:

> 
> === modified file 'src/ai/ai_help_structs.h'
> --- src/ai/ai_help_structs.h  2016-03-12 20:06:24 +
> +++ src/ai/ai_help_structs.h  2016-04-02 16:49:56 +
> @@ -351,8 +350,8 @@
>   uint16_t mines_percent;  // % of res it can mine
>   uint32_t current_stats;
>  
> - std::vector inputs;
> - std::vector outputs;
> + std::vector inputs;
> + std::vector outputs;

What _is_ a DescriptionIndex? Found it in src/logic/widelands.h but where is 
the object this index is ued for?

>   std::vector critical_building_material;
>  
>   bool produces_building_material;
> 
> === modified file 
> 'src/editor/ui_menus/editor_tool_change_resources_options_menu.cc'
> --- src/editor/ui_menus/editor_tool_change_resources_options_menu.cc  
> 2016-03-14 18:10:09 +
> +++ src/editor/ui_menus/editor_tool_change_resources_options_menu.cc  
> 2016-04-02 16:49:56 +
> @@ -197,10 +197,7 @@
>   case Change_By_Increase: change_by += change_by < 63; break;
>   case Change_By_Decrease: change_by -= 1 < change_by;  break;
>   caseSet_To_Increase: set_to+= set_to< 63; break;
> - caseSet_To_Decrease: set_to-= 0 < set_to; break;
> - default:
> - NEVER_HERE();
> - break;
> + caseSet_To_Decrease: set_to-= 0 < set_to;

-=0 // thats a noop?

>   }
>   increase_tool_.set_change_by(change_by);
>   increase_tool_.decrease_tool().set_change_by(change_by);
> 
> === modified file 'src/logic/map_objects/bob.cc'
> --- src/logic/map_objects/bob.cc  2016-03-31 06:56:28 +
> +++ src/logic/map_objects/bob.cc  2016-04-02 16:49:56 +
> @@ -712,7 +712,7 @@
>   // Slowing down a ship if two or more on same spot
>   // Using probability of 1/8 and pausing it for 5, 10 or 15 seconds
>   if (game.logic_rand() % 8 == 0) {
> - if (upcast(Ship, ship, this)) {
> + if (is_a(Ship, this)) {

Ah, much better!

>   Map& map = game.map();
>   const uint32_t ships_count
>   = 
> map.find_bobs(Widelands::Area(get_position(), 0), 
> nullptr, FindBobShip());
> 
> === modified file 'src/scripting/lua_globals.cc'
> --- src/scripting/lua_globals.cc  2015-02-28 17:43:48 +
> +++ src/scripting/lua_globals.cc  2016-04-02 16:49:56 +
> @@ -104,7 +104,6 @@
>   case LUA_TTHREAD:
>   case LUA_TLIGHTUSERDATA:
>   report_error(L, "Cannot format the 
> given type %s at index %i", lua_typename(L, i), i);
> - break;

This looks wrong, perhaps:
NEVER_HERE(); // as report_error will never return
would be better?

>  
>   default:
>   {
> 
> === modified file 'src/ui_basic/multilinetextarea.h'
> --- src/ui_basic/multilinetextarea.h  2016-03-25 17:01:05 +
> +++ src/ui_basic/multilinetextarea.h  2016-04-02 16:49:56 +
> @@ -74,7 +74,7 @@
>  
>   /**
>* This prepares a non-richtext text for rendering. It escapes the 
> source text and
> -  * turns \n into  tags as needed, then creates the richtext style 
> wrappers.
> +  * turns '\\n' into '' tags as needed, then creates the richtext 
> style wrappers.

Thx, that was the most annyoing one

>*/
>   std::string make_richtext();
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

Hello Gun: changes look good to me,
Compiles on OSX without any new Issues.
Will try to play this on trunk today.

(Thanks for playing the Triangle :-)


-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

Uhm, the fix for bug-1562332 was somehow reverted, I will add it in this branch 
again
-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

I got a crash in
0   widelands   0x00010d0cfa40 
FullscreenMenuInternetLobby::fill_client_list(std::__1::vector const*) + 2336 (vector:641)
1   widelands   0x00010d0cf0e2 FullscreenMenuInternetLobby::think() + 146 
(internet_lobby.cc:182)

but this is the same on trunk.

@SirVer I will remove the std:move now and then merge with trunk again.

-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/travis-clang-warnings into lp:widelands

[Klaus Halfmann]

Ready for merge, any objections?

SirVer, can you appove?
-- 
https://code.launchpad.net/~widelands-dev/widelands/travis-clang-warnings/+merge/290697
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/travis-clang-warnings.

___
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/revised_maps into lp:widelands

[Klaus Halfmann]

Review: Approve playing

OK, I tried (got tired) of the nile, I think we should merrge this
now. Next time we should do this for single maps only.

I check some code, but not all, as I have no Idea what to check for.

So lets get his merged so we can check for single problems/bugs later.
If there are any conflicts to be resolved, I may try this next week perhaps.

-- 
https://code.launchpad.net/~widelands-dev/widelands/revised_maps/+merge/289704
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/revised_maps.

___
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-1562332 into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/bug-1562332 
into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1562332 in widelands: "Crash in 
FullscreenMenuInternetLobby::fill_client_list"
  https://bugs.launchpad.net/widelands/+bug/1562332

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1562332/+merge/290794

This just fiexes the crash.

Some more general Issues are adessed in #1434875
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1562332 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/bug-1542821-internet-exceptions into lp:widelands

[Klaus Halfmann]

Review: Approve compiled, tested, code review

That was easy, I still do not find my way around in that Internet code.

Thx for fixing this one.

Showing the version was _very_ good idea, too
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1542821-internet-exceptions/+merge/292698
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1542821-internet-exceptions.

___
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_1571009_work_area_radius into lp:widelands

[Klaus Halfmann]

Review: Resubmit

Hello Gun, I reviewd that comment in workarea_info.h again.
Its just that I do not fully understand the usage of that map, yet.
So if you know what stings are used, please add them.

As of the implementation of std:map this will be correct, I guess.
But it usually is a abd idea to depend on some implemenation that
may change. OTOH the hashcode of integer may not change ever again.

Example: From Java JDK1.3 to 1.4 the Implementation of String.hashCode()
was changed. Some code had implicit dependencies on the order of Elements
in string based maps an therefore started to fail.

Please feel free to change the comment to your liking.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius.

___
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-1228811-observer-minimap into lp:widelands

[Klaus Halfmann]

Review: Approve compile, codereview, manual test

Had two computer players play while I watched and saved two times.
When trying to load the game I saw a minimap (terrain only).
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1228811-observer-minimap/+merge/292716
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1228811-observer-minimap.

___
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-website/encyclopedia into lp:widelands-website

[Klaus Halfmann]

kaputtnik: waht you need is a, so called, headless mode.

Either SDL can provide this (No idea how o do this, thiugh).
Or you can set um some X-Server thats works on some local
Bitmaps, which may be sufficent for your case.

Perhpas it is not needed so set up any graphics fo this case,
but the code forgot about that?

Just my 2cents of Knowledge
-- 
https://code.launchpad.net/~widelands-dev/widelands-website/encyclopedia/+merge/287410
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/port-clearance into lp:widelands

[Klaus Halfmann]

Review: Approve code review, compile, test

Works for me.

The effect may still be void as the trees may have grown again after the port 
was placed. (WHich would be some nic tactic to stop a port by some enemy).


-- 
https://code.launchpad.net/~widelands-dev/widelands/port-clearance/+merge/295236
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/port-clearance.

___
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-1418154-collectors-teams into lp:widelands

[Klaus Halfmann]

Review: Approve compile, test, codereviw

Ok, statistics are fine now too,
you may still include the gold in the
status message (e.g. I increased my Gold Production 
an gained some 100 points this way)
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1418154-collectors-teams/+merge/294702
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1418154-collectors-teams.

___
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/fix_tut1_destroy_quarries into lp:widelands

[Klaus Halfmann]

Review: Approve compile, play

This was an easy one :-)
-- 
https://code.launchpad.net/~widelands-dev/widelands/fix_tut1_destroy_quarries/+merge/294699
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/fix_tut1_destroy_quarries.

___
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-1418154-collectors-teams into lp:widelands

[Klaus Halfmann]

A small nit: After the "you have lost message" I get a last status message, 
thats OK.

But it claims "The Game will end in ." instead of perhaps "The games is 
finished",
oder "The Game will end in 0 minutes" or even "The game was finished 1 Minute 
ago".

It may be ok (for the statistic as well) to continue reporting after the 
Win/Loose
message. In my case I may have tried some tricks to increase my score.

The Messages where consistent and readable (I will think about fairness some 
other time :-). 

No going to check the missing statistics with another game...
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1418154-collectors-teams/+merge/294702
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1418154-collectors-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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1418154-collectors-teams into lp:widelands

[Klaus Halfmann]

About the gold: it does _not_ appear in the status message, thats the bug.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1418154-collectors-teams/+merge/294702
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1418154-collectors-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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1418154-collectors-teams into lp:widelands

[Klaus Halfmann]

Mhh, I am missing some Gold Points in the calcualtions.

every team has som 12 extra point at the beginning that do not sum up.
Is there some resource calculated, but not reported?

I found https://wl.widelands.org/wiki/GameHelpSinglePlayer/

and there Gold is mentioned as well, as every Player starts with 4 colds
this would match.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1418154-collectors-teams/+merge/294702
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1418154-collectors-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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/gcc531 into lp:widelands

[Klaus Halfmann]

Review: Approve

OK, for me with _my_ change, but gcc may complain again?
-- 
https://code.launchpad.net/~widelands-dev/widelands/gcc531/+merge/293478
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/gcc531.

___
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/gcc531 into lp:widelands

[Klaus Halfmann]

Review: Needs Information codereview, compile

I now get some (new?) clang warning, see inline comments.
Shall I commit the change?

Diff comments:

> === modified file 'src/logic/queue_cmd_factory.cc'
> --- src/logic/queue_cmd_factory.cc2016-01-18 05:12:51 +
> +++ src/logic/queue_cmd_factory.cc2016-05-01 10:05:57 +
> @@ -112,7 +112,7 @@
>   case QueueCommandTypes::kReplaySyncRead:
>   case QueueCommandTypes::kReplayEnd:
>   case QueueCommandTypes::kNone:
> - throw wexception("Unknown Queue_Cmd_Id in file: %u", id);
> + throw wexception("Unknown Queue_Cmd_Id in file: %u", 
> static_cast(id));
>   }

Mhh, there only really good way to ourput an enum is some kind of toString() 
method,
But I do not know if C++ has some defaults here :-(. But this is an exceptional 
case :-)

>   NEVER_HERE();
>  }
> 
> === modified file 'src/map_io/map_players_view_packet.cc'
> --- src/map_io/map_players_view_packet.cc 2016-02-16 10:27:23 +
> +++ src/map_io/map_players_view_packet.cc 2016-05-01 10:05:57 +
> @@ -991,12 +991,12 @@
>   }
>  
>   //  edges
> - if (!bl_seen & (f_everseen | 
> bl_everseen))
> + if (!(bl_seen) & (f_everseen | 
> bl_everseen))
>   
> roads_file.unsigned_8(f_player_field.road_sw());
> - if (!br_seen & (f_everseen | 
> br_everseen))
> + if (!(br_seen) & (f_everseen | 
> br_everseen))
>   
> roads_file.unsigned_8(f_player_field.road_se());
> - if (!r_seen & (f_everseen |  
> r_everseen))
> - 
> roads_file.unsigned_8(f_player_field.road_e ());
> + if (!(r_seen) & (f_everseen | 
> r_everseen))
> + 
> roads_file.unsigned_8(f_player_field.road_e());

clang now complains:
src/map_io/map_players_view_packet.cc:994: Old C-Style cast. Change to 
static_cast(var) or similar!
src/map_io/map_players_view_packet.cc:996: Old C-Style cast. Change to 
static_cast(var) or similar!
src/map_io/map_players_view_packet.cc:998: Old C-Style cast. Change to 
static_cast(var) or similar!

Mhh, what does gcc complain about? As all these are boolean this code might be 
better with && and ||.
Shall I check in the change?

>   }
>  
>   //  geologic survey


-- 
https://code.launchpad.net/~widelands-dev/widelands/gcc531/+merge/293478
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/gcc531.

___
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/fh1_multiline_textarea into lp:widelands

[Klaus Halfmann]

Uhm, to late, already branched it.
/me will try to read first, next time :-)
-- 
https://code.launchpad.net/~widelands-dev/widelands/fh1_multiline_textarea/+merge/292033
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/fh1_multiline_textarea 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

[Klaus Halfmann]

Review: Approve compile / regression / review / check f1-help

Fine for me.
-- 
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_1571009_work_area_radius into lp:widelands

[Klaus Halfmann]

Thanks for the hints

Diff comments:

> 
> === modified file 'src/scripting/lua_map.cc'
> --- src/scripting/lua_map.cc  2016-04-11 06:45:29 +
> +++ src/scripting/lua_map.cc  2016-04-16 12:42:55 +
> @@ -1871,10 +1871,16 @@
>  /* RST
>   .. attribute:: workarea_radius
>  
> - (RO) the workarea_radius of the building as an int.
> + (RO) the first workarea_radius of the building as an 
> int, 
> + 0 in case bulding has no workareas
>  */
>  int LuaBuildingDescription::get_workarea_radius(lua_State * L) {
> - lua_pushinteger(L, get()->workarea_info_.begin()->first);
> +const WorkareaInfo& workareaInfo = get()->workarea_info_;

Mhh, Sirver prefers 3 spaces, and there is no consistent handling of this, yet.

> +if (!workareaInfo.empty()) {
> +lua_pushinteger(L, workareaInfo.begin()->first);
> +} else {
> +lua_pushinteger(L, 0);

Thhaks for the tip, im not firm with that lua-stuff and tried not pushing 
anything (which failed).
I will try that now

> +}
>   return 1;
>  }
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius 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/bug_1571009_work_area_radius into lp:widelands

[Klaus Halfmann]

hope I fixed the codecheck issues now.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius 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/bug_1571009_work_area_radius into lp:widelands

[Klaus Halfmann]

Whats the Problem with Appveyor? 
I did not find the Problem.
Did the build take to long?
Did I cancel it?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius.

___
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-1560085-save-suggested-teams-display into lp:widelands

[Klaus Halfmann]

Review: Approve review, compile, test

Fixes the bug and improves variable names, fine.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1560085-save-suggested-teams-display/+merge/291852
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1560085-save-suggested-teams-display.

___
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_1571009_work_area_radius into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1571009 in widelands: "Work area radius: 45xxx in bzr7962[trunk]"
  https://bugs.launchpad.net/widelands/+bug/1571009

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066

This fixes 1571009 and adds some more Docs for the WorkareaInfo.

Im not usre about the Lua-Mapping, perhaps returning some kind of null might be 
better?
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands.
=== modified file 'src/logic/map_objects/tribes/workarea_info.h'
--- src/logic/map_objects/tribes/workarea_info.h	2015-11-28 22:29:26 +
+++ src/logic/map_objects/tribes/workarea_info.h	2016-04-16 12:42:55 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005, 2008 by the Widelands Development Team
+ * Copyright (C) 2005-2016 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -25,9 +25,22 @@
 #include 
 #include 
 
-//  This type is used to store information about workareas. It stores radii and
-//  for each radius a set of strings. Each string contains a description of an
-//  activity (or similar) that can be performed within the radius.
+/** The WorkareaInfo stores radii and for each radius a set of strings. 
+ *
+ * A Workarea is a "circle" around a building that this building affects
+ * or is needed by this building, e.g. Areas for Mines, Fields of a Farm.
+ * Worareads are shown on the Map when clicking on a building.
+ *
+ * Each string contains a description of an  activity (or similar) i
+ * that can be performed within the radius.
+ *
+ * TODO(Hasi50): In fact this complex idea of a workarea is not used.
+ * I do knot know of any building that has different sizes of workareas
+ * during its liftimer. LuaBuildingDescription::get_workarea_radiu does not use it
+ * and the GUI does not show it. 
+ *
+ * So we should just use a simple unit8 perhaps?
+ */
 using WorkareaInfo = std::map<uint32_t, std::set>;
 
 #endif  // end of include guard: WL_LOGIC_MAP_OBJECTS_TRIBES_WORKAREA_INFO_H

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2016-04-11 06:45:29 +
+++ src/scripting/lua_map.cc	2016-04-16 12:42:55 +
@@ -1871,10 +1871,16 @@
 /* RST
 	.. attribute:: workarea_radius
 
-			(RO) the workarea_radius of the building as an int.
+			(RO) the first workarea_radius of the building as an int, 
+ 0 in case bulding has no workareas
 */
 int LuaBuildingDescription::get_workarea_radius(lua_State * L) {
-	lua_pushinteger(L, get()->workarea_info_.begin()->first);
+const WorkareaInfo& workareaInfo = get()->workarea_info_;
+if (!workareaInfo.empty()) {
+lua_pushinteger(L, workareaInfo.begin()->first);
+} else {
+lua_pushinteger(L, 0);
+}
 	return 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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1302593-result-screen into lp:widelands

[Klaus Halfmann]

Review: Approve compile, test, code review

Testprotocol -- Testing this was not that easy ...

* bzr7981[bug-1302593-result-screen]
* Playing "Impact" with "Autocrat" as Barbarian versus (no AI) 
* Imperial and Atlanters
* Saved as "Test1" before defeating Imperial
* Saved as "Test2" before defeating Atlanter
* defeated Atlanter -> game won, was in Pause
* Saved as "Test3"
* Quit
* Loaded "Test3", Congratulation was still visible
* Loaded "Test2"
* defeated Atlanter -> game won, was in Pause
? Did I miss somthing?

So this test is complete.

Still the code improvements proposed by Miroslav are valid.
Im not that sure about some details, as I have no complete idea how saveloading 
works.

* I will test this again once someone has changed the code.
* Shall I upload those testgames to the original Bug, so someone lese may test?


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1302593-result-screen/+merge/293521
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1302593-result-screen.

___
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/seafaring-final-objective into lp:widelands

[Klaus Halfmann]

Review: Needs Information compile, play, codereview

OK, I merged with trunk, which allowed me to load the saved game again.
I now was able to play the tutorial (nice one) till the end, but
the last objective still remains. As of the code I cannot see
how this should happen. Must I start the tutorial form the beginning 
again?
-- 
https://code.launchpad.net/~widelands-dev/widelands/seafaring-final-objective/+merge/301060
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/seafaring-final-objective.

___
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/seafaring-final-objective into lp:widelands

[Klaus Halfmann]

Review: Approve compile, play

OK, finally it works.
Now please tell me why that simple code change did this trick.

Can I merge this wia launchpad now, or must someone else do this?
-- 
https://code.launchpad.net/~widelands-dev/widelands/seafaring-final-objective/+merge/301060
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/seafaring-final-objective.

___
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-1588063 into lp:widelands

[Klaus Halfmann]

Review: Approve compile, review, test

OK, Tested some "evil" charaters. On OSX '`´&$()[]{} are allowed while |/":* 
are not (incomplete).

Maybe we should filter these as well, to avoid Filenames with bad effects on 
the Commandline?
OTOH the average user will not not use such characters, will he/she?

Code LGTM, but I can test OSX only, so we need some Windows test, too.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1588063/+merge/301636
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1588063.

___
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-1588063 into lp:widelands

[Klaus Halfmann]

Testing on OSX:

* removed .widelands folder
* :bug-1588063$ ./widelands --editor
* Created some random map and saved it
* found ./widelands/maps/My_Maps, OK
* Copied some selfmade map into ./widelands/maps
ls -R maps
Crossriver.wmf  My_Maps

maps/My_Maps:
Test1608558.wmf

* I can open Crossriver.wmf in the Editor, is merged into the list of Maps.

I now will restore my old .widelands folder an try some evil characters
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1588063/+merge/301636
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1588063.

___
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:~7010622-q/widelands/topple-production-logic-2 into lp:widelands

[Klaus Halfmann]

Code llooks +/- equal for all tribes, lets do some testing.
-- 
https://code.launchpad.net/~7010622-q/widelands/topple-production-logic-2/+merge/301477
Your team Widelands Developers is requested to review the proposed merge of 
lp:~7010622-q/widelands/topple-production-logic-2 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/appveyor_linking_memory into lp:widelands

[Klaus Halfmann]

I actually did the same (but less sophisticated) for the Raspi
https://wl.widelands.org/forum/topic/2031/
but with some different flags, mmh.

What can I do to test this? (other then waiting for appveyor)

-- 
https://code.launchpad.net/~widelands-dev/widelands/appveyor_linking_memory/+merge/302275
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/appveyor_linking_memory.

___
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/seafaring-final-objective into lp:widelands

[Klaus Halfmann]

Review: Needs Information compile, plase

Uhhm, when testing this I got:

terrain: Terrain 'summer_meadow1' exists in map, not in world!

After loading any saved file. Can we merge in some fix for this?
-- 
https://code.launchpad.net/~widelands-dev/widelands/seafaring-final-objective/+merge/301060
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/seafaring-final-objective.

___
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-1346965-pointer-table into lp:widelands

[Klaus Halfmann]

Review: Approve compile, test, code review

Looks good to me,
compiled and opend all the tables I coud think of and sorted them.
Found no crashes or Anomalies 

gcc 5 in travis just needed to long?
Appveyor debug x64: could not read symbols: Memory exhausted.

These are no real Code Problems, can we avoid these?

-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1346965-pointer-table/+merge/300974
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1346965-pointer-table.

___
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-1625502-tips-in-help into lp:widelands

[Klaus Halfmann]

Code LGTM, one nit about  but as this HTML is not in the wild,
it may not be worth changing.
Will compile / test this now
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1625502-tips-in-help/+merge/312969
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1625502-tips-in-help.

___
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/update_copyright_script into lp:widelands

[Klaus Halfmann]

Review: Approve test, use

Testing is all fine

update_copyright_script$ ./utils/update_copyright.py 
Usage: update_copyright.py 
brombeere:update_copyright_script $ ./utils/update_copyright.py 2017
Updating copyright year to: 2017 ... lots of dots ... done.

brombeere:update_copyright_script $ bzr diff
.. lots of changes like ...
- * Copyright (C) 2006, 2008-2010 by the Widelands Development Team
+ * Copyright (C) 2006-2017 by the Widelands Development Team

all sane
/usr/bin/env python -> Python 3.4.6

Tried with python2.6 and python2.7 with the same result.

I did no re-(test) fix_formatting.py. Is this applied automatically or manually?





-- 
https://code.launchpad.net/~widelands-dev/widelands/update_copyright_script/+merge/315352
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/update_copyright_script.

___
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-1627537-window-mouserelease into lp:widelands

[Klaus Halfmann]

I wonder, SirVer did you test that that bug is fixed?

The code is ok for me, too.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1627537-window-mouserelease/+merge/313700
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1627537-window-mouserelease.

___
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/align-align into lp:widelands

[Klaus Halfmann]

Review: Resubmit

Gun: are you sure about changing alignement of Textarea, WordWrap and 
RenderTarget to HAlign only?

void Textarea::draw() uses H and VAlignent, so text _can_ be centered 
vertically.
Same for WordWrap:draw() and RenderTarget::blit().

Doing this will introduce a semantic change we should adress in a seperate 
branch.
Can we go with my changes now and adress your wishes later?

I am not done checking all your comments, but lets check if Travis and Appveyor 
are happy now.
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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/align-align into lp:widelands

[Klaus Halfmann]

FullscreenWindow will need Tiling { kNone, kHorizontal, kVertical }
and FullscreenWindow actually needs the all the Align enums.

I will try what I can make of it.
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/align-align 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/align-align into lp:widelands

[Klaus Halfmann]

Checked all you comments now, most are related to Textarea, WordWrap or 
RenderTarget.
Change two others as you suggested.
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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-1658489-epedition-tab into lp:widelands

[Klaus Halfmann]

Review: Resubmit

Can this go in now?

I am vaction now and would like to start some other things...
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1658489-epedition-tab/+merge/317047
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1658489-epedition-tab.

___
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/align-align into lp:widelands

[Klaus Halfmann]

The alinment of the chatbox in the Mutiplayerlobby is now right,
must check the code for MultilineTextarea again. 
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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/align-align into lp:widelands

[Klaus Halfmann]

So now Align is HAlign and VAlign at the same time.
The originla Align is reprocuded by struct Alignment with halign and valign.

OK, we left the Era of 8bit, so no need to conserve that Memory.

Currently compiling but not in paralell to have some time for checking.
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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/align-align into lp:widelands

[Klaus Halfmann]

Review: Approve compile, test

Fine for me, played 30m around some savegame.

Did not do a complete codereview
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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/align-align into lp:widelands

[Klaus Halfmann]

I will try to merge this with trunk, looks like there are no conflicts (!)
-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/align-align.

___
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/fh1-sub2div into lp:widelands

[Klaus Halfmann]

Code looks straight forward 1,$ s/sub/div/ ;-)

will comipile this and take a look
-- 
https://code.launchpad.net/~widelands-dev/widelands/fh1-sub2div/+merge/318138
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/fh1-sub2div 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/fh1-sub2div into lp:widelands

[Klaus Halfmann]

Review: Approve compile, test, review

Works for me, lets work for Travis/Appvoyer, for completness
-- 
https://code.launchpad.net/~widelands-dev/widelands/fh1-sub2div/+merge/318138
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/fh1-sub2div.

___
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/hasimusic into lp:widelands

[Klaus Halfmann]

Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/hasimusic into 
lp:widelands.

Requested reviews:
  toptopple (7010622-q)
Related bugs:
  Bug #1549963 in widelands: "Proposal for a new Game music"
  https://bugs.launchpad.net/widelands/+bug/1549963
  Bug #1556620 in widelands: "Silkweavers Song"
  https://bugs.launchpad.net/widelands/+bug/1556620
  Bug #1568929 in widelands: "Widelands March"
  https://bugs.launchpad.net/widelands/+bug/1568929

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/hasimusic/+merge/317892

After my Music was lingering around for some time now,
I now added it to the Game. As no real player plays with
music on this will not make widelands worse.

It will however change the first impression, as I used my
Widelands-Song as intro.
-- 
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/hasimusic.
=== renamed file 'data/music/intro_00.ogg' => 'data/music/ingame_23.ogg'
=== added file 'data/music/ingame_24.ogg'
Binary files data/music/ingame_24.ogg	1970-01-01 00:00:00 + and data/music/ingame_24.ogg	2017-02-21 18:15:17 + differ
=== added file 'data/music/ingame_25.ogg'
Binary files data/music/ingame_25.ogg	1970-01-01 00:00:00 + and data/music/ingame_25.ogg	2017-02-21 18:15:17 + differ
=== added file 'data/music/intro_00.ogg'
Binary files data/music/intro_00.ogg	1970-01-01 00:00:00 + and data/music/intro_00.ogg	2017-02-21 18:15:17 + differ
___
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/align-align into lp:widelands

[Klaus Halfmann]

Hello Gun, thnaks for the first review,
I was not sure where to use one dimension HAlign and where two Align,
I will follow your suggestions and change the sigantures of the funktions 
involved.

About the Bitmask: I will extract an anonymous enum and put them there,
as they are needed for bitwiese fucntions (slice Align into HAlign / VALign).

About using Hex: As these numbers should be seen as bits I like to use hex to
emphasise this. 

Will start the next plow through the code tomorrow ...

-- 
https://code.launchpad.net/~widelands-dev/widelands/align-align/+merge/317871
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/align-align 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