Merge lp:~widelands-dev/widelands/fix-bug-1208130 into lp:widelands

[cghislai]

I suggest merging Teppo's fix to the desync issue with soldiers preference. It has been tested

[SirVer (sirver)]

This has conflicts with master - hard to see what you actually changed.

review: Needs Fixing

[Teppo Mäenpää (kxq)]

Hi, I have now merged trunk/6711 to this branch for a re-review.

About the merge conflict:
There was a line saying " if ( !workarea_info.size() ) return ; show_workarea();"
code checker complained about it.
I had changed that to " if ( workarea_info.empty() ) return ; show_workarea();"
In trunk it was changed to " if ( !workarea_info.empty() ) return ; show_workarea();"
As these two modifications applied to the same line were not the same, there was a conflict.

I prefer my fix in the conflicting line better, as in current trunk the workarea is shown only when there is no workarea, while here it works, as before. Of course, my view could be biased.

Do you see it a bad thing, if people like me piggy-back some compiler/codechecker warning fixes to other fixes?

=================================

What was changed:

- Removed dead code from buildingwindow (methods void act_prefer_rookies() and act_prefer_heroes() )
- Changed the soldier list so that only player can click the preference button
- Changed the soldier to use the command relay function instead of directly changing the militarysite, to avoid desyncs in multiplayer games.
- Something I did not do, presumably Charly also figured out to keep the preference-tab up-to-date for observers ?

review: Needs Resubmitting

[Teppo Mäenpää (kxq)]

About the last bullet in the list of changes above: Yes, the preference stays in synch now -- I forgot to fix that line in my comment. It takes me about one hour to compile widelands, so I wrote the above text while waiting for miracles to happen.. SirVer, please re-review this branch, and merge if you find it OK.

[SirVer (sirver)]

One hour!!?!?! Oh wow. Are you using ccache? http://ccache.samba.org/ . It takes me < 5 minutes to make a fresh build after a make clean.

I will review as soons as I find some energy for this. Thanks for working on this Teppo.

[Teppo Mäenpää (kxq)]

Widelands is the most heavy-weight application I have ever used (off-work, that is), by a wide margin. My computer is just slow, CPU-wise. It only hurts when working with widelands, in all other cases I prefer my no-moving-parts-except-in-the-keyboard approach. It is faster than raspberrypi.. Normal computers make horrible sounds. To be honest, I have considered buying a normal computer just to speed WL (both game and compilation) up.

I do not think that any software solutions can overcome that. Turning some optimizations off in debug builds might help.

[Nasenbaer (nasenbaer)]

Looks perfectly fine to me :).
Thank you for the fix Teppo!

review: Approve

=== modified file 'src/wui/buildingwindow.cc'
--- src/wui/buildingwindow.cc	2013-08-10 16:57:13 +0000
+++ src/wui/buildingwindow.cc	2013-08-11 03:39:50 +0000
@@ -423,24 +423,6 @@
 	die();
 }
 
-void Building_Window::act_prefer_rookies()
-{
-	if (is_a(Widelands::MilitarySite, &m_building))
-		igbase().game().send_player_militarysite_set_soldier_preference
-			(m_building, Widelands::MilitarySite::kPrefersRookies);
-
-	die();
-}
-
-void
-Building_Window::act_prefer_heroes()
-{
-	if (is_a(Widelands::MilitarySite, &m_building))
-		igbase().game().send_player_militarysite_set_soldier_preference
-			(m_building, Widelands::MilitarySite::kPrefersHeroes);
-
-	die();
-}
 
 /**
 ===============
@@ -501,7 +483,7 @@
 	} else {
 		workarea_info = m_building.descr().m_workarea_info;
 	}
-	if (!workarea_info.empty()) {
+	if (workarea_info.empty()) {
 		return;
 	}
 	m_workarea_job_id = igbase().show_work_area(workarea_info, m_building.get_position());
 
=== modified file 'src/wui/buildingwindow.h'
--- src/wui/buildingwindow.h	2013-08-02 14:20:21 +0000
+++ src/wui/buildingwindow.h	2013-08-11 03:39:50 +0000
@@ -66,8 +66,6 @@
 	void toggle_workarea();
 	void configure_workarea_button();
 	void act_start_stop();
-	void act_prefer_rookies();
-	void act_prefer_heroes();
 	void act_start_or_cancel_expedition();
 	void act_enhance(Widelands::Building_Index);
 	void clicked_goto();
 
=== modified file 'src/wui/soldierlist.cc'
--- src/wui/soldierlist.cc	2013-07-27 10:35:55 +0000
+++ src/wui/soldierlist.cc	2013-08-11 03:39:50 +0000
@@ -368,6 +368,7 @@
 	void mouseover(const Soldier * soldier);
 	void eject(const Soldier * soldier);
 	void set_soldier_preference(int32_t changed_to);
+	void think();
 
 	Interactive_GameBase & m_igb;
 	Widelands::Building & m_building;
@@ -406,6 +407,7 @@
 
 	UI::Box * buttons = new UI::Box(this, 0, 0, UI::Box::Horizontal);
 
+	bool can_act = m_igb.can_act(m_building.owner().player_number());
 	if (upcast(Widelands::MilitarySite, ms, &building)) {
 		m_soldier_preference.add_button
 			(buttons, Point(0, 0), g_gr->images().get("pics/prefer_rookies.png"), _("Prefer Rookies"));
@@ -421,8 +423,12 @@
 		if (ms->get_soldier_preference() == Widelands::MilitarySite::kPrefersHeroes) {
 			m_soldier_preference.set_state(1);
 		}
-		m_soldier_preference.changedto.connect
-			(boost::bind(&SoldierList::set_soldier_preference, this, _1));
+		if (can_act) {
+			m_soldier_preference.changedto.connect
+				(boost::bind(&SoldierList::set_soldier_preference, this, _1));
+		} else {
+			m_soldier_preference.set_enabled(false);
+		}
 	}
 	buttons->add_inf_space();
 	buttons->add
@@ -437,6 +443,28 @@
 	return *dynamic_cast<SoldierControl *>(&m_building);
 }
 
+void SoldierList::think()
+{
+	// Only update the soldiers pref radio if player is spectator
+	if (m_igb.can_act(m_building.owner().player_number())) {
+		return;
+	}
+	if (upcast(Widelands::MilitarySite, ms, &m_building)) {
+		switch (ms->get_soldier_preference()) {
+			case Widelands::MilitarySite::kPrefersRookies:
+				m_soldier_preference.set_state(0);
+				break;
+			case Widelands::MilitarySite::kPrefersHeroes:
+				m_soldier_preference.set_state(1);
+				break;
+			default:
+				m_soldier_preference.set_state(-1);
+				break;
+		}
+	}
+}
+
+
 void SoldierList::mouseover(const Soldier * soldier)
 {
 	if (!soldier) {
@@ -474,8 +502,10 @@
 void SoldierList::set_soldier_preference(int32_t changed_to) {
 	upcast(Widelands::MilitarySite, ms, &m_building);
 	assert(ms);
-	ms->set_soldier_preference
-		(changed_to == 0 ? Widelands::MilitarySite::kPrefersRookies : Widelands::MilitarySite::kPrefersHeroes);
+	m_igb.game().send_player_militarysite_set_soldier_preference
+		(m_building, changed_to == 0 ?
+			Widelands::MilitarySite::kPrefersRookies:
+			Widelands::MilitarySite::kPrefersHeroes);
 }
 
 UI::Panel * create_soldier_list