Merge lp:~widelands-dev/widelands/reduce_overlay_manager_use3 into lp:widelands

[SirVer]

Draw build help slope indicators in the draw routine directly, not using the FieldOverlayManager.

[SirVer (sirver)]

For testing: this changes the way the help icons for road buildings are displayed. I mean the red, yellow and green indicators while building a road that shows the steepness of the road.

[GunChleoc (gunchleoc)]

Code LGTM, 2 nits. Needs testing.

Is there a reason you can't
remove enum class OverlayLevel {} right now? If the answer is "yes",
ignore this comment, otherwise remove.

The roads that are displayed
while a road is being build. -> built.

Sorry for the review format,
just a quick check-in during lunch break ;)

[SirVer (sirver)]

> Is there a reason you can't
remove enum class OverlayLevel {} right now? If the answer is "yes",
ignore this comment, otherwise remove.

No, it could be removed. but since it is part of the public API of the class it would entail further changes. Since the whole class is deleted in the next change after this one is merged (already done, just waiting for this to get in) I did not want to do the work.

> while a road is being build. -> built.

done.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2661. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/274208112.
Appveyor build 2482. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_reduce_overlay_manager_use3-2482.

[GunChleoc (gunchleoc)]

Sounds good to me :)

[kaputtnik (franku)]

Works when testing a release build :-)

Did also ran all regression tests.

review: Approve (testing)

[SirVer (sirver)]

Thanks!

@bunnybot merge

=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc	2017-09-04 15:02:05 +0000
+++ src/ui_basic/window.cc	2017-09-11 19:19:33 +0000
@@ -399,7 +399,7 @@
 	return true;
 }
 
-bool Window::handle_mousewheel(uint32_t which, int32_t x, int32_t y) {
+bool Window::handle_mousewheel(uint32_t, int32_t, int32_t) {
 	// Mouse wheel events should not propagate to objects below us, so we claim
 	// that they have been handled.
 	return true;

=== modified file 'src/wui/field_overlay_manager.h'
--- src/wui/field_overlay_manager.h	2017-08-28 20:44:08 +0000
+++ src/wui/field_overlay_manager.h	2017-09-11 19:19:33 +0000
@@ -51,10 +51,8 @@
 // Levels for the overlay registers. This defines in which order they will be
 // drawn. Buildhelp is special and has the value 5, i.e. every smaller will be
 // drawn below the buildhelp, everything higher above.
-enum class OverlayLevel {
-	kWorkAreaPreview = 0,
-	kRoadBuildSlope = 8,
-};
+// TODO(sirver): no longer used. remove.
+enum class OverlayLevel {};
 
 struct FieldOverlayManager {
 	/// A unique id identifying a registered overlay.

=== modified file 'src/wui/interactive_base.cc'
--- src/wui/interactive_base.cc	2017-09-11 16:57:25 +0000
+++ src/wui/interactive_base.cc	2017-09-11 19:19:33 +0000
@@ -83,7 +83,6 @@
      lastframe_(SDL_GetTicks()),
      frametime_(0),
      avg_usframetime_(0),
-     road_buildhelp_overlay_jobid_(0),
      buildroad_(nullptr),
      road_build_player_(0),
      unique_window_handler_(new UniqueWindowHandler()),
@@ -670,7 +669,8 @@
 */
 void InteractiveBase::roadb_add_overlay() {
 	assert(buildroad_);
-	assert(road_building_preview_.empty());
+	assert(road_building_overlays_.road_previews.empty());
+	assert(road_building_overlays_.steepness_indicators.empty());
 
 	const Map& map = egbase().map();
 
@@ -685,14 +685,12 @@
 			dir = Widelands::get_reverse_dir(dir);
 		}
 		int32_t const shift = 2 * (dir - Widelands::WALK_E);
-		road_building_preview_[c] |= (Widelands::RoadType::kNormal << shift);
+		road_building_overlays_.road_previews[c] |= (Widelands::RoadType::kNormal << shift);
 	}
 
 	// build hints
 	Widelands::FCoords endpos = map.get_fcoords(buildroad_->get_end());
 
-	assert(!road_buildhelp_overlay_jobid_);
-	road_buildhelp_overlay_jobid_ = field_overlay_manager_->next_overlay_id();
 	for (int32_t dir = 1; dir <= 6; ++dir) {
 		Widelands::FCoords neighb;
 		int32_t caps;
@@ -733,10 +731,7 @@
 			name = "images/wui/overlays/roadb_yellow.png";
 		else
 			name = "images/wui/overlays/roadb_red.png";
-
-		field_overlay_manager_->register_overlay(neighb, g_gr->images().get(name),
-		                                         OverlayLevel::kRoadBuildSlope, Vector2i::invalid(),
-		                                         road_buildhelp_overlay_jobid_);
+		road_building_overlays_.steepness_indicators[neighb] = g_gr->images().get(name);
 	}
 }
 
@@ -747,14 +742,8 @@
 */
 void InteractiveBase::roadb_remove_overlay() {
 	assert(buildroad_);
-
-	road_building_preview_.clear();
-
-	// build hints
-	if (road_buildhelp_overlay_jobid_) {
-		field_overlay_manager_->remove_overlay(road_buildhelp_overlay_jobid_);
-	}
-	road_buildhelp_overlay_jobid_ = 0;
+	road_building_overlays_.road_previews.clear();
+	road_building_overlays_.steepness_indicators.clear();
 }
 
 bool InteractiveBase::handle_key(bool const down, SDL_Keysym const code) {

=== modified file 'src/wui/interactive_base.h'
--- src/wui/interactive_base.h	2017-09-10 18:17:55 +0000
+++ src/wui/interactive_base.h	2017-09-11 19:19:33 +0000
@@ -63,6 +63,15 @@
 		dfDebug = 4,           ///< general debugging info
 	};
 
+	// Overlays displayed while a road is under construction.
+	struct RoadBuildingOverlays {
+		// The roads that are displayed while a road is being built. They are not
+		// yet logically in the game, but need to be displayed for the user as
+		// visual guide. The data type is the same as for Field::road.
+		std::map<Widelands::Coords, uint8_t> road_previews;
+		std::map<Widelands::Coords, const Image*> steepness_indicators;
+	};
+
 	// Manages all UniqueWindows.
 	UniqueWindowHandler& unique_windows();
 
@@ -162,8 +171,8 @@
 	// Sets the landmark for the keyboard 'key' to 'point'
 	void set_landmark(size_t key, const MapView::View& view);
 
-	const std::map<Widelands::Coords, uint8_t>& road_building_preview() const {
-		return road_building_preview_;
+	const RoadBuildingOverlays& road_building_overlays() const {
+		return road_building_overlays_;
 	}
 
 	MapView* map_view() {
@@ -267,10 +276,7 @@
 	// coordinate that the building that shows the work area is positioned.
 	std::map<Widelands::Coords, const WorkareaInfo*> work_area_previews_;
 
-	// The roads that are displayed while a road is being built. They are not
-	// yet logically in the game, but need to be displayed for the user as
-	// visual guide. The data type is the same as for Field::road.
-	std::map<Widelands::Coords, uint8_t> road_building_preview_;
+	RoadBuildingOverlays road_building_overlays_;
 
 	std::unique_ptr<Notifications::Subscriber<GraphicResolutionChanged>>
 	   graphic_resolution_changed_subscriber_;
@@ -281,7 +287,6 @@
 	uint32_t frametime_;        //  in millseconds
 	uint32_t avg_usframetime_;  //  in microseconds!
 
-	FieldOverlayManager::OverlayId road_buildhelp_overlay_jobid_;
 	Widelands::CoordPath* buildroad_;  //  path for the new road
 	Widelands::PlayerNumber road_build_player_;
 

=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc	2017-09-03 13:03:56 +0000
+++ src/wui/interactive_player.cc	2017-09-11 19:19:33 +0000
@@ -330,7 +330,7 @@
 	const uint32_t gametime = gbase.get_gametime();
 
 	auto* fields_to_draw = given_map_view->draw_terrain(gbase, dst);
-	const auto& roads_preview = road_building_preview();
+	const auto& road_building = road_building_overlays();
 	const std::map<Widelands::Coords, const Image*> work_area_overlays = get_work_area_overlays(map);
 
 	for (size_t idx = 0; idx < fields_to_draw->size(); ++idx) {
@@ -355,8 +355,8 @@
 
 		// Add road building overlays if applicable.
 		{
-			const auto it = roads_preview.find(f->fcoords);
-			if (it != roads_preview.end()) {
+			const auto it = road_building.road_previews.find(f->fcoords);
+			if (it != road_building.road_previews.end()) {
 				f->roads |= it->second;
 			}
 		}
@@ -398,6 +398,14 @@
 			const Image* pic = get_sel_picture();
 			blit_overlay(pic, Vector2i(pic->width() / 2, pic->height() / 2));
 		}
+
+		// Draw road building slopes.
+		{
+			const auto it = road_building.steepness_indicators.find(f->fcoords);
+			if (it != road_building.steepness_indicators.end()) {
+				blit_overlay(it->second, Vector2i(it->second->width() / 2, it->second->height() / 2));
+			}
+		}
 	}
 }
 

=== modified file 'src/wui/interactive_spectator.cc'
--- src/wui/interactive_spectator.cc	2017-08-31 20:58:02 +0000
+++ src/wui/interactive_spectator.cc	2017-09-11 19:19:33 +0000
@@ -110,7 +110,9 @@
 
 void InteractiveSpectator::draw_map_view(MapView* given_map_view, RenderTarget* dst) {
 	// A spectator cannot build roads.
-	assert(road_building_preview().empty());
+	assert(road_building_overlays().steepness_indicators.empty());
+	assert(road_building_overlays().road_previews.empty());
+
 	// In-game, selection can never be on triangles or have a radius.
 	assert(get_sel_radius() == 0);
 	assert(!get_sel_triangles());