widelands

Merge lp:~widelands-dev/widelands/bug-1191556-cancel-expedition into lp:widelands

  1. bug-1191556-cancel-expedition
  2. Merge into trunk
Proposed by GunChleoc
Status: Merged
Merged at revision: 7886
Proposed branch: lp:~widelands-dev/widelands/bug-1191556-cancel-expedition
Merge into: lp:widelands
Diff against target: 211 lines (+75/-27)
7 files modified
src/economy/expedition_bootstrap.cc (+3/-1)
src/economy/expedition_bootstrap.h (+11/-0)
src/notifications/note_ids.h (+1/-1)
src/ui_basic/tabpanel.cc (+13/-1)
src/ui_basic/tabpanel.h (+3/-0)
src/wui/buildingwindow.cc (+35/-24)
src/wui/buildingwindow.h (+9/-0)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1191556-cancel-expedition
Reviewer Review Type Date Requested Status
Klaus Halfmann Approve
TiborB Approve
Review via email: mp+288375@code.launchpad.net

Commit message

The "Cancel Expedition" button in Port windows will now toggle and remove the tab.

Description of the change

Using a new note and a new "remove_last_tab" function to fix the cancel expedition button in port windows.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 811. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/114584036.
Appveyor build 656. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1191556_cancel_expedition-656.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 822. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/114849725.
Appveyor build 656. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1191556_cancel_expedition-656.

Revision history for this message
TiborB (tiborb95) wrote :

Code LGTM, but testing is really needed here

review: Approve
Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

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

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for testing :)

If you have any ideas concerning refactoring - I am all ears! We could open bugs for those.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

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.

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

Yes, that crash sounds unrelated. Do you have any more information on the crash? It would be good to open a bug for it.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have copied the info into a new bug: https://bugs.launchpad.net/widelands/+bug/1557567

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/economy/expedition_bootstrap.cc'
2--- src/economy/expedition_bootstrap.cc 2016-01-18 19:35:25 +0000
3+++ src/economy/expedition_bootstrap.cc 2016-03-12 13:22:28 +0000
4@@ -160,8 +160,10 @@
5 workers_.clear();
6
7 // Update the user interface
8- if (upcast(InteractiveGameBase, igb, warehouse->owner().egbase().get_ibase()))
9+ if (upcast(InteractiveGameBase, igb, warehouse->owner().egbase().get_ibase())) {
10 warehouse->refresh_options(*igb);
11+ }
12+ Notifications::publish(NoteExpeditionCanceled(this));
13 }
14
15 void ExpeditionBootstrap::cleanup(EditorGameBase& /* egbase */) {
16
17=== modified file 'src/economy/expedition_bootstrap.h'
18--- src/economy/expedition_bootstrap.h 2015-11-28 22:29:26 +0000
19+++ src/economy/expedition_bootstrap.h 2016-03-12 13:22:28 +0000
20@@ -99,6 +99,17 @@
21 DISALLOW_COPY_AND_ASSIGN(ExpeditionBootstrap);
22 };
23
24+
25+struct NoteExpeditionCanceled {
26+ CAN_BE_SENT_AS_NOTE(NoteId::NoteExpeditionCanceled)
27+
28+ ExpeditionBootstrap* bootstrap;
29+
30+ NoteExpeditionCanceled(ExpeditionBootstrap* const init_bootstrap)
31+ : bootstrap(init_bootstrap) {
32+ }
33+};
34+
35 } // namespace Widelands
36
37 #endif // end of include guard: WL_ECONOMY_EXPEDITION_BOOTSTRAP_H
38
39=== modified file 'src/notifications/note_ids.h'
40--- src/notifications/note_ids.h 2016-01-14 22:09:24 +0000
41+++ src/notifications/note_ids.h 2016-03-12 13:22:28 +0000
42@@ -36,7 +36,7 @@
43 TrainingSiteSoldierTrained,
44 ShipMessage,
45 GraphicResolutionChanged,
46-
47+ NoteExpeditionCanceled
48 };
49
50 #endif // end of include guard: WL_NOTIFICATIONS_NOTE_IDS_H
51
52=== modified file 'src/ui_basic/tabpanel.cc'
53--- src/ui_basic/tabpanel.cc 2016-02-18 18:27:52 +0000
54+++ src/ui_basic/tabpanel.cc 2016-03-12 13:22:28 +0000
55@@ -254,12 +254,24 @@
56 }
57
58 /**
59- * Return the tab names in order
60+ * Return the tabs in order
61 */
62 const TabPanel::TabList & TabPanel::tabs() {
63 return tabs_;
64 }
65
66+bool TabPanel::remove_last_tab(const std::string& tabname) {
67+ if (tabs_.back()->get_name() == tabname) {
68+ tabs_.pop_back();
69+ if (active_ > tabs_.size() - 1) {
70+ active_ = 0ul;
71+ }
72+ update_desired_size();
73+ return true;
74+ }
75+ return false;
76+}
77+
78 /**
79 * Draw the buttons and the tab
80 */
81
82=== modified file 'src/ui_basic/tabpanel.h'
83--- src/ui_basic/tabpanel.h 2016-02-08 12:38:48 +0000
84+++ src/ui_basic/tabpanel.h 2016-03-12 13:22:28 +0000
85@@ -115,6 +115,9 @@
86 void activate(uint32_t idx);
87 void activate(const std::string &);
88 uint32_t active() {return active_;}
89+ // Removes the last tab if the 'tabname' matches. Returns whether a tab was removed.
90+ // We use the tabname as a safety precaution to prevent acidentally removing the wrong tab.
91+ bool remove_last_tab(const std::string& tabname);
92
93 boost::signals2::signal<void ()> sigclicked;
94
95
96=== modified file 'src/wui/buildingwindow.cc'
97--- src/wui/buildingwindow.cc 2016-02-18 18:27:52 +0000
98+++ src/wui/buildingwindow.cc 2016-03-12 13:22:28 +0000
99@@ -58,7 +58,8 @@
100 registry_(registry),
101 building_ (b),
102 workarea_overlay_id_(0),
103- avoid_fastclick_(false)
104+ avoid_fastclick_(false),
105+ expeditionbtn_(nullptr)
106 {
107 delete registry_;
108 registry_ = this;
109@@ -178,27 +179,20 @@
110 // Check if this is a port building and if yes show expedition button
111 if (upcast(Widelands::Warehouse const, warehouse, &building_)) {
112 if (Widelands::PortDock * pd = warehouse->get_portdock()) {
113- if (pd->expedition_started()) {
114- UI::Button * expeditionbtn =
115- new UI::Button
116- (capsbuttons, "cancel_expedition", 0, 0, 34, 34,
117- g_gr->images().get("images/ui_basic/but4.png"),
118- g_gr->images().get("images/wui/buildings/cancel_expedition.png"),
119- _("Cancel the expedition"));
120- expeditionbtn->sigclicked.connect
121- (boost::bind(&BuildingWindow::act_start_or_cancel_expedition, boost::ref(*this)));
122- capsbuttons->add(expeditionbtn, UI::Align::kHCenter);
123- } else {
124- UI::Button * expeditionbtn =
125- new UI::Button
126- (capsbuttons, "start_expedition", 0, 0, 34, 34,
127- g_gr->images().get("images/ui_basic/but4.png"),
128- g_gr->images().get("images/wui/buildings/start_expedition.png"),
129- _("Start an expedition"));
130- expeditionbtn->sigclicked.connect
131- (boost::bind(&BuildingWindow::act_start_or_cancel_expedition, boost::ref(*this)));
132- capsbuttons->add(expeditionbtn, UI::Align::kHCenter);
133- }
134+ expedition_canceled_subscriber_ =
135+ Notifications::subscribe<Widelands::NoteExpeditionCanceled>(
136+ [this](const Widelands::NoteExpeditionCanceled&) {
137+ update_expedition_button(true);
138+ });
139+ expeditionbtn_ =
140+ new UI::Button
141+ (capsbuttons, "start_or_cancel_expedition", 0, 0, 34, 34,
142+ g_gr->images().get("images/ui_basic/but4.png"),
143+ g_gr->images().get("images/wui/buildings/start_expedition.png"));
144+ update_expedition_button(!pd->expedition_started());
145+ expeditionbtn_->sigclicked.connect
146+ (boost::bind(&BuildingWindow::act_start_or_cancel_expedition, boost::ref(*this)));
147+ capsbuttons->add(expeditionbtn_, UI::Align::kHCenter);
148 }
149 }
150 else
151@@ -422,9 +416,13 @@
152 ===============
153 */
154 void BuildingWindow::act_start_or_cancel_expedition() {
155- if (upcast(Widelands::Warehouse const, warehouse, &building_))
156- if (warehouse->get_portdock())
157+ if (upcast(Widelands::Warehouse const, warehouse, &building_)) {
158+ if (warehouse->get_portdock()) {
159+ expeditionbtn_->set_enabled(false);
160 igbase().game().send_player_start_or_cancel_expedition(building_);
161+ }
162+ get_tabs()->activate("expedition_wares_queue");
163+ }
164
165 // No need to die here - as soon as the request is handled, the UI will get updated by the portdock
166 }
167@@ -538,3 +536,16 @@
168 {
169 igbase().move_view_to(building().get_position());
170 }
171+
172+void BuildingWindow::update_expedition_button(bool expedition_was_canceled) {
173+ assert(expeditionbtn_ != nullptr);
174+ if (expedition_was_canceled) {
175+ expeditionbtn_->set_tooltip(_("Start an expedition"));
176+ expeditionbtn_->set_pic(g_gr->images().get("images/wui/buildings/start_expedition.png"));
177+ tabs_->remove_last_tab("expedition_wares_queue");
178+ } else {
179+ expeditionbtn_->set_tooltip(_("Cancel the expedition"));
180+ expeditionbtn_->set_pic(g_gr->images().get("images/wui/buildings/cancel_expedition.png"));
181+ }
182+ expeditionbtn_->set_enabled(true);
183+}
184
185=== modified file 'src/wui/buildingwindow.h'
186--- src/wui/buildingwindow.h 2016-01-24 20:11:53 +0000
187+++ src/wui/buildingwindow.h 2016-03-12 13:22:28 +0000
188@@ -21,7 +21,9 @@
189 #define WL_WUI_BUILDINGWINDOW_H
190
191 #include <cstdlib>
192+#include <memory>
193
194+#include "economy/expedition_bootstrap.h"
195 #include "ui_basic/button.h"
196 #include "ui_basic/window.h"
197 #include "wui/field_overlay_manager.h"
198@@ -92,6 +94,13 @@
199
200 FieldOverlayManager::OverlayId workarea_overlay_id_;
201 bool avoid_fastclick_;
202+
203+ // For ports only.
204+ void update_expedition_button(bool expedition_was_canceled);
205+
206+ UI::Button * expeditionbtn_;
207+ std::unique_ptr<Notifications::Subscriber<Widelands::NoteExpeditionCanceled>>
208+ expedition_canceled_subscriber_;
209 };
210
211 #endif // end of include guard: WL_WUI_BUILDINGWINDOW_H

Subscribers

People subscribed via source and target branches

to status/vote changes: