widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/bug-1658489-epedition-tab into lp:widelands.

Requested reviews:
  kaputtnik (franku)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1658489-epedition-tab/+merge/317047

Fixed the bug in buildingwindow.cc + some minor changes.

Moral: Notifications are global. So when receiving one check if it is for you.
-- 
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1658489-epedition-tab.

=== modified file 'compile.sh'
--- compile.sh	2016-10-26 06:54:00 +0000
+++ compile.sh	2017-02-12 13:31:11 +0000
@@ -158,8 +158,8 @@
 bzr pull
 cd build
 $buildtool
-rm  ../VERSION || true
-rm  ../widelands || true
+rm  -f ../VERSION || true
+rm  -f ../widelands || true
 mv VERSION ../VERSION
 mv src/widelands ../widelands
 cd ..

=== modified file 'src/economy/expedition_bootstrap.cc'
--- src/economy/expedition_bootstrap.cc	2017-02-09 19:23:44 +0000
+++ src/economy/expedition_bootstrap.cc	2017-02-12 13:31:11 +0000
@@ -91,8 +91,12 @@
 		warehouse->refresh_options(*igb);
 }
 
+/**
+ * Cancel the Expediton by putting back all wares and workers.
+‚ */
 void ExpeditionBootstrap::cancel(Game& game) {
-	// Put all wares from the WaresQueues back into the warehouse
+
+    // Put all wares from the WaresQueues back into the warehouse
 	Warehouse* const warehouse = portdock_->get_warehouse();
 	for (std::unique_ptr<InputQueue>& iq : queues_) {
 		if (iq->get_type() == wwWARE) {
@@ -112,6 +116,8 @@
 	if (upcast(InteractiveGameBase, igb, warehouse->owner().egbase().get_ibase())) {
 		warehouse->refresh_options(*igb);
 	}
+
+    // This will result in bug-1658489 as some other Portdockwindow will be affexted, too
 	Notifications::publish(NoteExpeditionCanceled(this));
 }
 

=== modified file 'src/economy/expedition_bootstrap.h'
--- src/economy/expedition_bootstrap.h	2017-02-09 19:23:44 +0000
+++ src/economy/expedition_bootstrap.h	2017-02-12 13:31:11 +0000
@@ -38,9 +38,16 @@
 class Warehouse;
 class Worker;
 
-// Handles the mustering of workers and wares in a port for one Expedition. This
-// object is created in the port dock as soon as the start expedition button is
-// pressed. As soon as the ship is loaded, this object gets destroyed.
+/**
+ * Handles the mustering of workers and wares in a port for an Expedition.
+ *
+ * This object is created in the port dock as soon as the start expedition button is
+ * pressed. As soon as the ship is loaded, this object gets destroyed.
+ * In case the Expedition is ::cancel()ed the wares will be returned to the port dock.
+ * Its the responsibility of the Owner to finall dispose a canceled ExpeditionBootstrap.
+ *
+ */
+
 class ExpeditionBootstrap {
 public:
 	ExpeditionBootstrap(PortDock* const portdock);
@@ -53,9 +60,12 @@
 	// the corresponding warehouse.
 	void cancel(Game& game);
 
-	// Returns a list of workers and wares that are ready to go to an
-	// expedition. Ownership is transferred and the object is in an undefined
-	// state after this and must be deleted.
+	/**
+     * Returns a list of workers and wares that are ready to go to an expediton.
+     *
+	 * Ownership is transferred and the object is in an undefined
+	 * state after this and must be deleted.
+     */
 	void get_waiting_workers_and_wares(Game&,
 	                                   const TribeDescr&,
 	                                   std::vector<Worker*>* return_workers,
@@ -73,11 +83,18 @@
 	// Delete all wares we currently handle.
 	void cleanup(EditorGameBase& egbase);
 
-	// Save/Load this into a file. The actual data is stored in the buildingdata
-	// packet, and there in the warehouse data packet. The version parameter is
-	// the version number of the Warehouse packet.
-	void
-	load(Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, uint16_t version);
+	/** Load this from a file.
+     *
+     * The actual data is stored in the buildingdata
+	 * packet, and there in the warehouse data packet.
+     */
+	void load(Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, uint16_t version);
+
+    /** Save this into a file.
+     *
+     * The actual data is stored in the buildingdata
+     * packet, and there in the warehouse data packet.
+     */
 	void save(FileWrite& fw, Game& game, MapObjectSaver& mos);
 
 private:
@@ -87,6 +104,7 @@
 	// Tests if all wares for the expedition have arrived. If so, informs the portdock.
 	void is_ready(Game& game);
 
+    /** The Expedition is bootstapped here. */
 	PortDock* const portdock_;  // not owned
 	Economy* economy_;
 

=== modified file 'src/graphic/align.h'
--- src/graphic/align.h	2017-01-25 18:55:59 +0000
+++ src/graphic/align.h	2017-02-12 13:31:11 +0000
@@ -24,29 +24,41 @@
 
 namespace UI {
 
+/**
+ * This Enum is a binary mix of one-dimensional and two-dimensional alignments.
+ *
+ * bits 0,1 values 0,1,2,3  are horizontal
+ * bits 2,3 values 0,4,8,12 are vertical
+ *
+ * mixed aligenments are results of a binary | operation.
+ *
+ */
+
+ // TODO(klaus.halfmann): as this is not a real enum all compiler warnings about
+ // incomplete usage are useless.
+
 enum class Align {
-	kLeft = 0,
-	kHCenter = 1,
-	kRight = 2,
-	kHorizontal = 3,
-
-	kTop = 0,
-	kVCenter = 4,
-	kBottom = 8,
-	kVertical = 12,
-
-	kTopLeft = 0,
-	kCenterLeft = Align::kVCenter,
-	kBottomLeft = Align::kBottom,
-
-	kTopCenter = Align::kHCenter,
-	kCenter = Align::kHCenter | Align::kVCenter,
-	kBottomCenter = Align::kHCenter | Align::kBottom,
-
-	kTopRight = Align::kRight,
-	kCenterRight = Align::kRight | Align::kVCenter,
-
-	kBottomRight = Align::kRight | Align::kBottom,
+	kLeft       = 0x00,
+	kHCenter    = 0x01,
+	kRight      = 0x02,
+	kHorizontal = 0x03,
+
+	kTop        = 0x00,
+	kVCenter    = 0x04,
+	kBottom     = 0x08,
+	kVertical   = 0x0C,
+
+	kTopLeft        = kLeft | kTop,
+	kCenterLeft     = kLeft | kVCenter,
+	kBottomLeft     = kLeft | kBottom,
+
+    kTopCenter      = kHCenter | kTop,
+	kCenter         = kHCenter | kVCenter,
+	kBottomCenter   = kHCenter | kBottom,
+
+	kTopRight       = kRight | kTop,
+	kCenterRight    = kRight | kVCenter,
+	kBottomRight    = kRight | kBottom,
 };
 
 inline Align operator&(Align a, Align b) {

=== modified file 'src/notifications/notifications_impl.h'
--- src/notifications/notifications_impl.h	2017-01-25 18:55:59 +0000
+++ src/notifications/notifications_impl.h	2017-02-12 13:31:11 +0000
@@ -70,7 +70,8 @@
 
 	// Publishes 'message' to all subscribers.
 	template <typename T> void publish(const T& message) {
-		for (void* p_subscriber : note_id_to_subscribers_[T::note_id()]) {
+        std::list<void*> subscribers = note_id_to_subscribers_[T::note_id()];
+		for (void* p_subscriber : subscribers) {
 			Subscriber<T>* subscriber = static_cast<Subscriber<T>*>(p_subscriber);
 			subscriber->callback_(message);
 		}

=== modified file 'src/ui_basic/box.cc'
--- src/ui_basic/box.cc	2017-01-25 18:55:59 +0000
+++ src/ui_basic/box.cc	2017-02-12 13:31:11 +0000
@@ -356,6 +356,7 @@
 
 /**
  * Position the given item according to its parameters.
+ *
  * pos is the position relative to the parent in the direction of the
  * orientation axis.
 */
@@ -376,16 +377,15 @@
 			maxbreadth = get_inner_w();
 		}
 		switch (it.u.panel.align) {
-		case UI::Align::kHCenter:
-			breadth = (maxbreadth - breadth) / 2;
-			break;
+            case UI::Align::kHCenter:
+                breadth = (maxbreadth - breadth) >> 1;
+                break;
 
-		case UI::Align::kRight:
-			breadth = maxbreadth - breadth;
-			break;
-		case UI::Align::kLeft:
-		default:
-			breadth = 0;
+            case UI::Align::kRight:
+                breadth = maxbreadth - breadth;
+                break;
+            default: // UI::Align::left
+                breadth = 0;
 		}
 
 		if (orientation_ == Horizontal)

=== modified file 'src/wui/building_ui.cc'
--- src/wui/building_ui.cc	2017-01-25 18:55:59 +0000
+++ src/wui/building_ui.cc	2017-02-12 13:31:11 +0000
@@ -68,7 +68,6 @@
 	if (optionswindow_) {
 		Vector2i window_position = optionswindow_->get_pos();
 		hide_options();
-		show_options(igb, true);
-		optionswindow_->set_pos(window_position);
+		show_options(igb, true, window_position);
 	}
 }

=== modified file 'src/wui/buildingwindow.cc'
--- src/wui/buildingwindow.cc	2017-01-28 14:53:28 +0000
+++ src/wui/buildingwindow.cc	2017-02-12 13:31:11 +0000
@@ -159,9 +159,6 @@
 		// Check if this is a port building and if yes show expedition button
 		if (upcast(Widelands::Warehouse const, warehouse, &building_)) {
 			if (Widelands::PortDock* pd = warehouse->get_portdock()) {
-				expedition_canceled_subscriber_ =
-				   Notifications::subscribe<Widelands::NoteExpeditionCanceled>([this](
-				      const Widelands::NoteExpeditionCanceled&) { update_expedition_button(true); });
 				expeditionbtn_ =
 				   new UI::Button(capsbuttons, "start_or_cancel_expedition", 0, 0, 34, 34,
 				                  g_gr->images().get("images/ui_basic/but4.png"),
@@ -170,6 +167,15 @@
 				expeditionbtn_->sigclicked.connect(
 				   boost::bind(&BuildingWindow::act_start_or_cancel_expedition, boost::ref(*this)));
 				capsbuttons->add(expeditionbtn_, UI::Align::kHCenter);
+
+                expedition_canceled_subscriber_ =
+                    Notifications::subscribe<Widelands::NoteExpeditionCanceled>([this, pd](
+                    const Widelands::NoteExpeditionCanceled& canceld) {
+                        // Check this was not just any but our Expedition
+                        if (canceld.bootstrap == pd->expedition_bootstrap()) {
+                            update_expedition_button(true);
+                        }
+                    });
 			}
 		} else if (upcast(const Widelands::ProductionSite, productionsite, &building_)) {
 			if (!is_a(Widelands::MilitarySite, productionsite)) {