widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/reduce_overlay_manager_use into lp:widelands.

Commit message:
Draw resources and player starting positions overlays in the editor without the overlay manager.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1696064 in widelands: "Resources disappear in editor"
  https://bugs.launchpad.net/widelands/+bug/1696064

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/reduce_overlay_manager_use/+merge/329807
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/reduce_overlay_manager_use into lp:widelands.

=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc	2017-08-28 12:44:08 +0000
+++ src/editor/editorinteractive.cc	2017-08-29 12:21:52 +0000
@@ -63,27 +63,6 @@
 	egbase->tribes();
 }
 
-// Updates the resources overlays after a field has changed.
-void update_resource_overlay(const Widelands::NoteFieldResourceChanged& note,
-                             const Widelands::World& world,
-                             FieldOverlayManager* field_overlay_manager) {
-	//  Ok, we're doing something. First remove the current overlays.
-	if (note.old_resource != Widelands::kNoResource) {
-		const std::string str = world.get_resource(note.old_resource)->editor_image(note.old_amount);
-		const Image* pic = g_gr->images().get(str);
-		field_overlay_manager->remove_overlay(note.fc, pic);
-	}
-
-	const auto amount = note.fc.field->get_resources_amount();
-	const auto resource_type = note.fc.field->get_resources();
-	if (amount > 0 && resource_type != Widelands::kNoResource) {
-		const std::string str =
-		   world.get_resource(note.fc.field->get_resources())->editor_image(amount);
-		const Image* pic = g_gr->images().get(str);
-		field_overlay_manager->register_overlay(note.fc, pic, OverlayLevel::kResource);
-	}
-}
-
 }  // namespace
 
 EditorInteractive::EditorInteractive(Widelands::EditorGameBase& e)
@@ -168,43 +147,9 @@
 		map_clicked(node_and_triangle, false);
 	});
 
-	// Subscribe to changes of the resource type on a field..
-	field_resource_changed_subscriber_ =
-	   Notifications::subscribe<Widelands::NoteFieldResourceChanged>(
-	      [this](const Widelands::NoteFieldResourceChanged& note) {
-		      update_resource_overlay(note, egbase().world(), mutable_field_overlay_manager());
-		   });
-
 	minimap_registry().minimap_type = MiniMapType::kStaticMap;
 }
 
-void EditorInteractive::register_overlays() {
-	Widelands::Map* map = egbase().mutable_map();
-
-	//  Starting locations
-	Widelands::PlayerNumber const nr_players = map->get_nrplayers();
-	assert(nr_players <= kMaxPlayers);
-	iterate_player_numbers(p, nr_players) {
-		if (Widelands::Coords const sp = map->get_starting_pos(p)) {
-			tools_->set_starting_pos.set_starting_pos(*this, p, sp, map);
-		}
-	}
-
-	//  Resources: we do not calculate default resources, therefore we do not
-	//  expect to meet them here.
-	Widelands::Extent const extent = map->extent();
-	iterate_Map_FCoords(*map, extent, fc) {
-		if (uint8_t const amount = fc.field->get_resources_amount()) {
-			const std::string& immname =
-			   egbase().world().get_resource(fc.field->get_resources())->editor_image(amount);
-			if (immname.size()) {
-				mutable_field_overlay_manager()->register_overlay(
-				   fc, g_gr->images().get(immname), OverlayLevel::kResource);
-			}
-		}
-	}
-}
-
 void EditorInteractive::load(const std::string& filename) {
 	assert(filename.size());
 
@@ -318,6 +263,18 @@
 	const float scale = 1.f / map_view()->view().zoom;
 	const uint32_t gametime = ebase.get_gametime();
 
+	// The map provides a mapping from player number to Coords, while we require
+	// the inverse here. We construct this, but this is done on every frame and
+	// therefore potentially expensive - though it never showed up in any of my
+	// profiles. We could change the Map should this become a bottleneck, since
+	// plrnum -> coords is needed less often.
+	const auto& map = ebase.map();
+	std::map<Widelands::Coords, int> starting_positions;
+	for (int i = 1; i <= map.get_nrplayers(); ++i) {
+		starting_positions[map.get_starting_pos(i)] = i;
+	}
+
+	const auto& world = ebase.world();
 	for (size_t idx = 0; idx < fields_to_draw->size(); ++idx) {
 		const FieldsToDraw::Field& field = fields_to_draw->at(idx);
 		if (draw_immovables_) {
@@ -334,15 +291,37 @@
 			}
 		}
 
+		const auto blit_overlay = [&dst, &field, scale](const Image* pic, const Vector2i& hotspot) {
+			dst.blitrect_scale(Rectf(field.rendertarget_pixel - hotspot.cast<float>() * scale,
+			                         pic->width() * scale, pic->height() * scale),
+			                   pic, Recti(0, 0, pic->width(), pic->height()), 1.f,
+			                   BlendMode::UseAlpha);
+		};
+
+		// Draw resource overlay.
+		uint8_t const amount = field.fcoords.field->get_resources_amount();
+		if (draw_resources_ && amount > 0) {
+			const std::string& immname =
+			   world.get_resource(field.fcoords.field->get_resources())->editor_image(amount);
+			if (!immname.empty()) {
+				const auto* pic = g_gr->images().get(immname);
+				blit_overlay(pic, Vector2i(pic->width() / 2, pic->height() / 2));
+			}
+		}
+
 		// TODO(sirver): Do not use the field_overlay_manager, instead draw the
 		// overlays we are interested in here directly.
-		field_overlay_manager().foreach_overlay(
-		   field.fcoords, [&dst, &field, scale](const Image* pic, const Vector2i& hotspot) {
-			   dst.blitrect_scale(Rectf(field.rendertarget_pixel - hotspot.cast<float>() * scale,
-			                            pic->width() * scale, pic->height() * scale),
-			                      pic, Recti(0, 0, pic->width(), pic->height()), 1.f,
-			                      BlendMode::UseAlpha);
-			});
+		field_overlay_manager().foreach_overlay(field.fcoords, blit_overlay);
+
+		// Draw the player starting position overlays.
+		const auto it = starting_positions.find(field.fcoords);
+		if (it != starting_positions.end()) {
+			const Image* player_image =
+			   playercolor_image(it->second - 1, "images/players/player_position.png");
+			assert(player_image != nullptr);
+			constexpr int kStartingPosHotspotY = 55;
+			blit_overlay(player_image, Vector2i(player_image->width() / 2, kStartingPosHotspotY));
+		}
 	}
 }
 
@@ -382,10 +361,8 @@
 }
 
 void EditorInteractive::toggle_resources() {
-	auto* overlay_manager = mutable_field_overlay_manager();
-	const bool value = !overlay_manager->is_enabled(OverlayLevel::kResource);
-	overlay_manager->set_enabled(OverlayLevel::kResource, value);
-	toggle_resources_->set_perm_pressed(value);
+	draw_resources_ = !draw_resources_;
+	toggle_resources_->set_perm_pressed(draw_resources_);
 }
 
 void EditorInteractive::toggle_immovables() {
@@ -706,7 +683,6 @@
 	}
 
 	mutable_field_overlay_manager()->remove_all_overlays();
-	register_overlays();
 }
 
 EditorInteractive::Tools* EditorInteractive::tools() {

=== modified file 'src/editor/editorinteractive.h'
--- src/editor/editorinteractive.h	2017-08-20 16:12:28 +0000
+++ src/editor/editorinteractive.h	2017-08-29 12:21:52 +0000
@@ -150,9 +150,6 @@
 		void const* object;
 	};
 
-	// Registers the overlays for player starting positions.
-	void register_overlays();
-
 	void on_buildhelp_changed(const bool value) override;
 
 	void toggle_resources();
@@ -165,8 +162,6 @@
 	uint32_t realtime_;
 	bool is_painting_;
 
-	std::unique_ptr<Notifications::Subscriber<Widelands::NoteFieldResourceChanged>>
-	   field_resource_changed_subscriber_;
 	UI::UniqueWindow::Registry toolmenu_;
 
 	UI::UniqueWindow::Registry toolsizemenu_;
@@ -190,6 +185,7 @@
 	std::unique_ptr<Tools> tools_;
 	std::unique_ptr<EditorHistory> history_;
 
+	bool draw_resources_ = true;
 	bool draw_immovables_ = true;
 	bool draw_bobs_ = true;
 };

=== modified file 'src/editor/tools/set_starting_pos_tool.cc'
--- src/editor/tools/set_starting_pos_tool.cc	2017-08-13 18:38:27 +0000
+++ src/editor/tools/set_starting_pos_tool.cc	2017-08-29 12:21:52 +0000
@@ -55,14 +55,13 @@
 	return 0;
 }
 
-EditorSetStartingPosTool::EditorSetStartingPosTool()
-   : EditorTool(*this, *this, false), overlay_ids_(kMaxPlayers, 0) {
+EditorSetStartingPosTool::EditorSetStartingPosTool() : EditorTool(*this, *this, false) {
 	current_player_ = 1;
 }
 
 int32_t EditorSetStartingPosTool::handle_click_impl(const Widelands::World&,
                                                     const Widelands::NodeAndTriangle<>& center,
-                                                    EditorInteractive& eia,
+                                                    EditorInteractive&,
                                                     EditorActionArgs*,
                                                     Widelands::Map* map) {
 	assert(0 <= center.node.x);
@@ -80,35 +79,12 @@
 
 		//  check if field is valid
 		if (editor_tool_set_starting_pos_callback(map->get_fcoords(center.node), *map)) {
-			set_starting_pos(eia, current_player_, center.node, map);
+			map->set_starting_pos(current_player_, center.node);
 		}
 	}
 	return 1;
 }
 
-void EditorSetStartingPosTool::set_starting_pos(EditorInteractive& eia,
-                                                Widelands::PlayerNumber plnum,
-                                                const Widelands::Coords& c,
-                                                Widelands::Map* map) {
-	FieldOverlayManager* overlay_manager = eia.mutable_field_overlay_manager();
-	//  remove old overlay if any
-	overlay_manager->remove_overlay(overlay_ids_.at(plnum - 1));
-
-	//  add new overlay
-	FieldOverlayManager::OverlayId overlay_id = overlay_manager->next_overlay_id();
-	overlay_ids_[plnum - 1] = overlay_id;
-
-	const Image* player_image = playercolor_image(plnum - 1, "images/players/player_position.png");
-	assert(player_image);
-
-	overlay_manager->register_overlay(c, player_image, OverlayLevel::kPlayerStartingPosition,
-	                                  Vector2i(player_image->width() / 2, STARTING_POS_HOTSPOT_Y),
-	                                  overlay_id);
-
-	//  set new player pos
-	map->set_starting_pos(plnum, c);
-}
-
 Widelands::PlayerNumber EditorSetStartingPosTool::get_current_player() const {
 	return current_player_;
 }

=== modified file 'src/editor/tools/set_starting_pos_tool.h'
--- src/editor/tools/set_starting_pos_tool.h	2017-08-13 18:02:53 +0000
+++ src/editor/tools/set_starting_pos_tool.h	2017-08-29 12:21:52 +0000
@@ -30,7 +30,6 @@
 // How much place should be left around a player position
 // where no other player can start
 #define MIN_PLACE_AROUND_PLAYERS 24
-#define STARTING_POS_HOTSPOT_Y 55
 
 /// Sets the starting position of players.
 struct EditorSetStartingPosTool : public EditorTool {
@@ -50,13 +49,6 @@
 	bool has_size_one() const override {
 		return true;
 	}
-	void set_starting_pos(EditorInteractive& eia,
-	                      Widelands::PlayerNumber plnum,
-	                      const Widelands::Coords& c,
-	                      Widelands::Map* map);
-
-private:
-	std::vector<FieldOverlayManager::OverlayId> overlay_ids_;
 };
 
 int32_t editor_tool_set_starting_pos_callback(const Widelands::FCoords& c, Widelands::Map& map);

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2017-08-20 17:45:42 +0000
+++ src/logic/map.cc	2017-08-29 12:21:52 +0000
@@ -1816,14 +1816,9 @@
 	if (resource_type == Widelands::kNoResource) {
 		amount = 0;
 	}
-	const auto note = NoteFieldResourceChanged{
-	   c, c.field->resources, c.field->initial_res_amount, c.field->res_amount,
-	};
-
 	c.field->resources = resource_type;
 	c.field->initial_res_amount = amount;
 	c.field->res_amount = amount;
-	Notifications::publish(note);
 }
 
 void Map::set_resources(const FCoords& c, ResourceAmount amount) {
@@ -1831,11 +1826,7 @@
 	if (c.field->resources == Widelands::kNoResource) {
 		return;
 	}
-	const auto note = NoteFieldResourceChanged{
-	   c, c.field->resources, c.field->initial_res_amount, c.field->res_amount,
-	};
 	c.field->res_amount = amount;
-	Notifications::publish(note);
 }
 
 void Map::clear_resources(const FCoords& c) {

=== modified file 'src/logic/map.h'
--- src/logic/map.h	2017-08-20 17:45:42 +0000
+++ src/logic/map.h	2017-08-29 12:21:52 +0000
@@ -74,16 +74,6 @@
 	MapIndex map_index;
 };
 
-/// Send when the resource of a field is changed.
-struct NoteFieldResourceChanged {
-	CAN_BE_SENT_AS_NOTE(NoteId::FieldResourceTypeChanged)
-
-	FCoords fc;
-	DescriptionIndex old_resource;
-	ResourceAmount old_initial_amount;
-	ResourceAmount old_amount;
-};
-
 struct ImmovableFound {
 	BaseImmovable* object;
 	Coords coords;

=== modified file 'src/wui/field_overlay_manager.h'
--- src/wui/field_overlay_manager.h	2017-08-15 17:37:57 +0000
+++ src/wui/field_overlay_manager.h	2017-08-29 12:21:52 +0000
@@ -53,10 +53,8 @@
 // drawn below the buildhelp, everything higher above.
 enum class OverlayLevel {
 	kWorkAreaPreview = 0,
-	kResource = 4,
 	kSelection = 7,
 	kRoadBuildSlope = 8,
-	kPlayerStartingPosition = 9,
 };
 
 struct FieldOverlayManager {