widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1721121-workers-invisible-wares into lp:widelands.

Commit message:
Restored ware hotspot to workers. Made some variables in WorkerDescr private and/or const.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1721121 in widelands: "Wares invisible when carried by workers (<>carriers)."
  https://bugs.launchpad.net/widelands/+bug/1721121

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1721121-workers-invisible-wares/+merge/343272
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1721121-workers-invisible-wares into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/carrier.cc'
--- src/logic/map_objects/tribes/carrier.cc	2018-04-07 16:59:00 +0000
+++ src/logic/map_objects/tribes/carrier.cc	2018-04-15 06:18:59 +0000
@@ -549,32 +549,10 @@
 	fw.signed_32(promised_pickup_to_);
 }
 
-void Carrier::draw_inner(const EditorGameBase& game,
-                         const Vector2f& point_on_dst,
-                         const float scale,
-                         RenderTarget* dst) const {
-	assert(get_owner() != nullptr);
-	Worker::draw_inner(game, point_on_dst, scale, dst);
-
-	if (WareInstance const* const carried_ware = get_carried_ware(game)) {
-		const RGBColor& player_color = get_owner()->get_playercolor();
-		const Vector2f hotspot = descr().get_ware_hotspot().cast<float>();
-		const Vector2f location(
-		   point_on_dst.x - hotspot.x * scale, point_on_dst.y - hotspot.y * scale);
-		dst->blit_animation(
-		   location, scale, carried_ware->descr().get_animation("idle"), 0, player_color);
-	}
-}
-
 CarrierDescr::CarrierDescr(const std::string& init_descname,
                            const LuaTable& table,
                            const EditorGameBase& egbase)
-   : WorkerDescr(init_descname, MapObjectType::CARRIER, table, egbase),
-     ware_hotspot_(Vector2i(0, 15)) {
-	if (table.has_key("ware_hotspot")) {
-		std::unique_ptr<LuaTable> items_table = table.get_table("ware_hotspot");
-		ware_hotspot_ = Vector2i(items_table->get_int(1), items_table->get_int(2));
-	}
+   : WorkerDescr(init_descname, MapObjectType::CARRIER, table, egbase) {
 }
 
 /**

=== modified file 'src/logic/map_objects/tribes/carrier.h'
--- src/logic/map_objects/tribes/carrier.h	2018-04-07 16:59:00 +0000
+++ src/logic/map_objects/tribes/carrier.h	2018-04-15 06:18:59 +0000
@@ -33,16 +33,10 @@
 	~CarrierDescr() override {
 	}
 
-	Vector2i get_ware_hotspot() const {
-		return ware_hotspot_;
-	}
-
 protected:
 	Bob& create_object() const override;
 
 private:
-	Vector2i ware_hotspot_;
-
 	DISALLOW_COPY_AND_ASSIGN(CarrierDescr);
 };
 
@@ -71,12 +65,6 @@
 
 	static Task const taskRoad;
 
-protected:
-	void draw_inner(const EditorGameBase& game,
-	                const Vector2f& point_on_dst,
-	                const float scale,
-	                RenderTarget* dst) const override;
-
 private:
 	void find_pending_ware(Game&);
 	int32_t find_closest_flag(Game&);

=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc	2018-04-07 16:59:00 +0000
+++ src/logic/map_objects/tribes/worker.cc	2018-04-15 06:18:59 +0000
@@ -2999,6 +2999,14 @@
 
 	dst->blit_animation(
 	   point_on_dst, scale, get_current_anim(), game.get_gametime() - get_animstart(), player_color);
+
+	if (WareInstance const* const carried_ware = get_carried_ware(game)) {
+		const Vector2f hotspot = descr().ware_hotspot().cast<float>();
+		const Vector2f location(
+		   point_on_dst.x - hotspot.x * scale, point_on_dst.y - hotspot.y * scale);
+		dst->blit_animation(
+		   location, scale, carried_ware->descr().get_animation("idle"), 0, player_color);
+	}
 }
 
 /**

=== modified file 'src/logic/map_objects/tribes/worker_descr.cc'
--- src/logic/map_objects/tribes/worker_descr.cc	2018-04-07 16:59:00 +0000
+++ src/logic/map_objects/tribes/worker_descr.cc	2018-04-15 06:18:59 +0000
@@ -40,9 +40,12 @@
                          const LuaTable& table,
                          const EditorGameBase& egbase)
    : BobDescr(init_descname, init_type, MapObjectDescr::OwnerType::kTribe, table),
-     buildable_(false),
-     needed_experience_(INVALID_INDEX),
-     becomes_(INVALID_INDEX),
+     ware_hotspot_(table.has_key("ware_hotspot") ? Vector2i(table.get_table("ware_hotspot")->get_int(1), table.get_table("ware_hotspot")->get_int(2)) : Vector2i(0, 15)),
+	  default_target_quantity_(table.has_key("default_target_quantity") ? table.get_int("default_target_quantity") : std::numeric_limits<uint32_t>::max()),
+     buildable_(table.has_key("buildcost")),
+	  // Read what the worker can become and the needed experience. If one of the keys is there, the other key must be there too. So, we cross the checks.
+     becomes_(table.has_key("experience") ? egbase.tribes().safe_worker_index(table.get_string("becomes")) : INVALID_INDEX),
+	  needed_experience_(table.has_key("becomes") ? table.get_int("experience") : INVALID_INDEX),
      egbase_(egbase) {
 	if (helptext_script().empty()) {
 		throw GameDataError("Worker %s has no helptext script", name().c_str());
@@ -54,7 +57,6 @@
 	std::unique_ptr<LuaTable> items_table;
 
 	if (table.has_key("buildcost")) {
-		buildable_ = true;
 		const Tribes& tribes = egbase_.tribes();
 		items_table = table.get_table("buildcost");
 		for (const std::string& key : items_table->keys<std::string>()) {
@@ -90,12 +92,6 @@
 		add_directional_animation(&walkload_anims_, "walkload");
 	}
 
-	// Read what the worker can become and the needed experience
-	if (table.has_key("becomes")) {
-		becomes_ = egbase_.tribes().safe_worker_index(table.get_string("becomes"));
-		needed_experience_ = table.get_int("experience");
-	}
-
 	// Read programs
 	if (table.has_key("programs")) {
 		std::unique_ptr<LuaTable> programs_table = table.get_table("programs");
@@ -114,11 +110,6 @@
 			}
 		}
 	}
-	if (table.has_key("default_target_quantity")) {
-		default_target_quantity_ = table.get_int("default_target_quantity");
-	} else {
-		default_target_quantity_ = std::numeric_limits<uint32_t>::max();
-	}
 }
 
 WorkerDescr::WorkerDescr(const std::string& init_descname,

=== modified file 'src/logic/map_objects/tribes/worker_descr.h'
--- src/logic/map_objects/tribes/worker_descr.h	2018-04-07 16:59:00 +0000
+++ src/logic/map_objects/tribes/worker_descr.h	2018-04-15 06:18:59 +0000
@@ -62,6 +62,10 @@
 		return buildcost_;
 	}
 
+	const Vector2i& ware_hotspot() const {
+		return ware_hotspot_;
+	}
+
 	/// How much of the worker type that an economy should store in warehouses.
 	/// The special value std::numeric_limits<uint32_t>::max() means that the
 	/// the target quantity of this ware type will never be checked and should
@@ -82,9 +86,6 @@
 			default_target_quantity_ = 1;
 	}
 
-	const DirAnimations& get_walk_anims() const {
-		return walk_anims_;
-	}
 	const DirAnimations& get_right_walk_anims(bool const carries_ware) const {
 		return carries_ware ? walkload_anims_ : walk_anims_;
 	}
@@ -116,26 +117,34 @@
 	}
 
 protected:
-	Quantity default_target_quantity_;
+	Programs programs_;
+
+private:
+	const Vector2i ware_hotspot_;
+
 	DirAnimations walk_anims_;
 	DirAnimations walkload_anims_;
-	bool buildable_;
+
+	Quantity default_target_quantity_;
+	const bool buildable_;
 	Buildcost buildcost_;
 
 	/**
+	 * Type that this worker can become, i.e. level up to (or null).
+	 */
+	const DescriptionIndex becomes_;
+
+	/**
 	 * Number of experience points required for leveling up,
 	 * or INVALID_INDEX if the worker cannot level up.
 	 */
-	int32_t needed_experience_;
-
-	/**
-	 * Type that this worker can become, i.e. level up to (or null).
-	 */
-	DescriptionIndex becomes_;
-	Programs programs_;
-	std::set<DescriptionIndex> employers_;  // Buildings where ths worker can work
-private:
+	const int32_t needed_experience_;
+
+	/// Buildings where this worker can work
+	std::set<DescriptionIndex> employers_;
+
 	const EditorGameBase& egbase_;
+
 	DISALLOW_COPY_AND_ASSIGN(WorkerDescr);
 };
 }