← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~amdat/widelands/HunterBug1407418 into lp:widelands

 

daAlx1 has proposed merging lp:~amdat/widelands/HunterBug1407418 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~amdat/widelands/HunterBug1407418/+merge/251689

solves Bug 1407418 Multiple Hunters hunt for same animal
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~amdat/widelands/HunterBug1407418 into lp:widelands.
=== modified file 'src/logic/immovable.cc'
--- src/logic/immovable.cc	2015-02-05 11:26:01 +0000
+++ src/logic/immovable.cc	2015-03-04 05:39:16 +0000
@@ -135,8 +135,8 @@
 */
 
 ImmovableProgram::ImmovableProgram(const std::string& directory,
-                                   Profile& prof,
-                                   const std::string& _name,
+								   Profile& prof,
+								   const std::string& _name,
 											  ImmovableDescr& immovable)
    : m_name(_name) {
 	Section& program_s = prof.get_safe_section(_name.c_str());
@@ -178,7 +178,7 @@
 }
 
 ImmovableProgram::ImmovableProgram(const std::string& init_name,
-                                   const std::vector<std::string>& lines,
+								   const std::vector<std::string>& lines,
 											  ImmovableDescr* immovable)
    : m_name(init_name) {
 	for (const std::string& line : lines) {
@@ -319,7 +319,7 @@
 	   world.editor_immovable_categories().get_index(table.get_string("editor_category"));
 	if (editor_category_index < 0) {
 		throw GameDataError("Unknown editor_category: %s\n",
-		                      table.get_string("editor_category").c_str());
+							  table.get_string("editor_category").c_str());
 	}
 	editor_category_ = world.editor_immovable_categories().get(editor_category_index);
 
@@ -407,8 +407,7 @@
 m_program_ptr (0),
 m_anim_construction_total(0),
 m_anim_construction_done(0),
-m_program_step(0),
-m_reserved_by_worker(false)
+m_program_step(0)
 {}
 
 Immovable::~Immovable()
@@ -575,28 +574,14 @@
 			 .str();
 		m_construct_string = as_uifont(m_construct_string);
 		dst.blit(pos - Point(0, 48),
-		         UI::g_fh1->render(m_construct_string),
-		         BlendMode::UseAlpha,
-		         UI::Align_Center);
+				 UI::g_fh1->render(m_construct_string),
+				 BlendMode::UseAlpha,
+				 UI::Align_Center);
 	}
 }
 
 
-/**
- * Returns whether this immovable was reserved by a worker.
- */
-bool Immovable::is_reserved_by_worker() const
-{
-	return m_reserved_by_worker;
-}
 
-/**
- * Change whether this immovable is marked as reserved by a worker.
- */
-void Immovable::set_reserved_by_worker(bool reserve)
-{
-	m_reserved_by_worker = reserve;
-}
 
 /**
  * Set the current action's data to \p data.
@@ -690,8 +675,10 @@
 
 	imm.m_program_step = fr.signed_32();
 
+	//TODO(daAlx1): reserved_by_worker is already dealt with in MapObject,
+	//so storing / loading twice is redundant but I keep it for compatability
 	if (version >= 3)
-		imm.m_reserved_by_worker = fr.unsigned_8();
+	imm.m_reserved_by_worker = fr.unsigned_8();
 
 	if (version >= 4) {
 		std::string dataname = fr.c_string();
@@ -753,6 +740,8 @@
 	fw.unsigned_32(m_program_ptr);
 	fw.signed_32(m_program_step);
 
+	//TODO(daAlx1):  reserved_by_worker is already dealt with in MapObject,
+	//so storing / loading twice is redundant, I keep it for compatability
 	fw.unsigned_8(m_reserved_by_worker);
 
 	if (m_action_data) {
@@ -1012,7 +1001,7 @@
 	const ImmovableDescr& descr = immovable.descr();
 
 	if (logic_rand_as_double(&game) <
-	    probability_to_grow(descr.terrain_affinity(), f, map, game.world().terrains())) {
+		probability_to_grow(descr.terrain_affinity(), f, map, game.world().terrains())) {
 		TribeDescr const* const owner_tribe = tribe ? descr.get_owner_tribe() : nullptr;
 		immovable.remove(game);  //  Now immovable is a dangling reference!
 		game.create_immovable(f, type_name, owner_tribe);
@@ -1112,7 +1101,7 @@
 	const ImmovableDescr& descr = immovable.descr();
 
 	if (logic_rand_as_double(&game) <
-	    probability_to_grow(descr.terrain_affinity(), f, map, game.world().terrains())) {
+		probability_to_grow(descr.terrain_affinity(), f, map, game.world().terrains())) {
 		// Seed a new tree.
 		MapFringeRegion<> mr(map, Area<>(f, 0));
 		uint32_t fringe_size = 0;
@@ -1127,10 +1116,10 @@
 
 		const FCoords new_location = map.get_fcoords(mr.location());
 		if (!new_location.field->get_immovable() &&
-		    (new_location.field->nodecaps() & MOVECAPS_WALK) &&
-		    logic_rand_as_double(&game) <
-		       probability_to_grow(
-		          descr.terrain_affinity(), new_location, map, game.world().terrains())) {
+			(new_location.field->nodecaps() & MOVECAPS_WALK) &&
+			logic_rand_as_double(&game) <
+			   probability_to_grow(
+				  descr.terrain_affinity(), new_location, map, game.world().terrains())) {
 			game.create_immovable(
 			   mr.location(), type_name, tribe ? immovable.descr().get_owner_tribe() : nullptr);
 		}

=== modified file 'src/logic/immovable.h'
--- src/logic/immovable.h	2015-02-08 18:16:41 +0000
+++ src/logic/immovable.h	2015-03-04 05:39:16 +0000
@@ -197,8 +197,6 @@
 	bool construct_ware(Game & game, WareIndex index);
 	bool construct_remaining_buildcost(Game & game, Buildcost * buildcost);
 
-	bool is_reserved_by_worker() const;
-	void set_reserved_by_worker(bool reserve);
 
 	void set_action_data(ImmovableActionData * data);
 	template<typename T>
@@ -248,14 +246,7 @@
 	 */
 	std::unique_ptr<ImmovableActionData> m_action_data;
 
-	/**
-	 * Immovables like trees are reserved by a worker that is walking
-	 * towards them, so that e.g. two lumberjacks don't attempt to
-	 * work on the same tree simultaneously.
-	 */
-	bool m_reserved_by_worker;
-
-	// Load/save support
+        // Load/save support
 protected:
 	struct Loader : public BaseImmovable::Loader {
 		void load(FileRead &, uint8_t version);

=== modified file 'src/logic/instances.cc'
--- src/logic/instances.cc	2014-12-01 21:28:21 +0000
+++ src/logic/instances.cc	2015-03-04 05:39:16 +0000
@@ -293,7 +293,7 @@
 }
 
 void MapObjectDescr::add_attributes(const std::vector<std::string>& attributes,
-                                      const std::set<uint32_t>& allowed_special) {
+									  const std::set<uint32_t>& allowed_special) {
 	for (const std::string& attribute : attributes) {
 		uint32_t const attrib = get_attribute_id(attribute);
 		if (attrib < MapObject::HIGHEST_FIXED_ATTRIBUTE) {
@@ -355,7 +355,10 @@
  * Zero-initialize a map object
  */
 MapObject::MapObject(const MapObjectDescr * const the_descr) :
-m_descr(the_descr), m_serial(0), m_logsink(nullptr)
+m_descr(the_descr),
+m_serial(0),
+m_logsink(nullptr),
+m_reserved_by_worker(false)
 {}
 
 
@@ -481,7 +484,18 @@
 }
 
 
-#define CURRENT_SAVEGAME_VERSION 1
+bool MapObject::is_reserved_by_worker() const
+{
+	return m_reserved_by_worker;
+}
+
+void MapObject::set_reserved_by_worker(bool reserve)
+{
+	m_reserved_by_worker = reserve;
+}
+
+
+#define CURRENT_SAVEGAME_VERSION 2
 
 /**
  * Load the entire data package from the given file.
@@ -501,7 +515,7 @@
 				("header is %u, expected %u", header, HeaderMapObject);
 
 		uint8_t const version = fr.unsigned_8();
-		if (version != CURRENT_SAVEGAME_VERSION)
+		if (version <= 0 || version > CURRENT_SAVEGAME_VERSION)
 			throw GameDataError("unknown/unhandled version %u", version);
 
 		Serial const serial = fr.unsigned_32();
@@ -510,6 +524,9 @@
 		} catch (const WException & e) {
 			throw wexception("%u: %s", serial, e.what());
 		}
+
+		if (version == CURRENT_SAVEGAME_VERSION)
+			get_object()->m_reserved_by_worker = fr.unsigned_8();
 	} catch (const WException & e) {
 		throw wexception("map object: %s", e.what());
 	}
@@ -551,6 +568,7 @@
 	fw.unsigned_8(CURRENT_SAVEGAME_VERSION);
 
 	fw.unsigned_32(mos.get_object_file_index(*this));
+	fw.unsigned_8(m_reserved_by_worker);
 }
 
 std::string to_string(const MapObjectType type) {

=== modified file 'src/logic/instances.h'
--- src/logic/instances.h	2014-11-30 18:49:38 +0000
+++ src/logic/instances.h	2015-03-04 05:39:16 +0000
@@ -90,8 +90,8 @@
 struct MapObjectDescr {
 
 	MapObjectDescr(const MapObjectType init_type,
-	                 const std::string& init_name,
-	                 const std::string& init_descname)
+					 const std::string& init_name,
+					 const std::string& init_descname)
 		: m_type(init_type), m_name(init_name), m_descname(init_descname) {
 	}
 	virtual ~MapObjectDescr() {m_anims.clear();}
@@ -278,6 +278,28 @@
 		HeaderFleet = 11,
 	};
 
+	protected:
+	/**
+	 * MapObjects like trees are reserved by a worker that is walking
+	 * towards them, so that e.g. two lumberjacks don't attempt to
+	 * work on the same tree simultaneously or two hunters try to hunt
+	 * the same animal.
+	 */
+	bool m_reserved_by_worker;
+
+	public:
+
+	/**
+	 * Returns whether this immovable was reserved by a worker.
+	 */
+	bool is_reserved_by_worker() const;
+
+	/**
+	 * Change whether this immovable is marked as reserved by a worker.
+	 */
+	void set_reserved_by_worker(bool reserve);
+
+
 	/**
 	 * Static load functions of derived classes will return a pointer to
 	 * a Loader class. The caller needs to call the virtual functions

=== modified file 'src/logic/worker.cc'
--- src/logic/worker.cc	2015-02-24 13:51:38 +0000
+++ src/logic/worker.cc	2015-03-04 05:39:16 +0000
@@ -117,8 +117,8 @@
 	if (static_cast<int8_t>(res) == -1) //  TODO(unknown): ARGH!!
 		throw GameDataError
 			(_
-			 	("should mine resource %s, which does not exist in world; tribe "
-			 	 "is not compatible with world"),
+				("should mine resource %s, which does not exist in world; tribe "
+				 "is not compatible with world"),
 			 action.sparam1.c_str());
 
 	// Select one of the fields randomly
@@ -222,8 +222,8 @@
 	if (static_cast<int8_t>(res) == -1) //  TODO(unknown): ARGH!!
 		throw GameDataError
 			(_
-			 	("should breed resource type %s, which does not exist in world; "
-			 	 "tribe is not compatible with world"),
+				("should breed resource type %s, which does not exist in world; "
+				 "tribe is not compatible with world"),
 			 action.sparam1.c_str());
 
 	// Select one of the fields randomly
@@ -394,8 +394,9 @@
 
 	Map & map = game.map();
 	Area<FCoords> area (map.get_fcoords(get_position()), 0);
+	bool found_reserved = false;
+
 	if (action.sparam1 == "immovable") {
-		bool found_reserved = false;
 
 		for (;; ++area.radius) {
 			if (action.iparam1 < area.radius) {
@@ -414,10 +415,10 @@
 			std::vector<ImmovableFound> list;
 			if (action.iparam2 < 0)
 				map.find_reachable_immovables
-					(area, &list, cstep);
+						(area, &list, cstep);
 			else
 				map.find_reachable_immovables
-					(area, &list, cstep, FindImmovableAttribute(action.iparam2));
+						(area, &list, cstep, FindImmovableAttribute(action.iparam2));
 
 			for (int idx = list.size() - 1; idx >= 0; idx--) {
 				if (upcast(Immovable, imm, list[idx].object)) {
@@ -439,7 +440,7 @@
 
 			if (!list.empty()) {
 				set_program_objvar
-					(game, state, list[game.logic_rand() % list.size()].object);
+						(game, state, list[game.logic_rand() % list.size()].object);
 				break;
 			}
 		}
@@ -448,10 +449,17 @@
 			if (action.iparam1 < area.radius) {
 				send_signal(game, "fail"); //  no object found, cannot run program
 				pop_task(game);
-				if (upcast(ProductionSite, productionsite, get_location(game)))
-					productionsite->notify_player(game, 30);
+				if (upcast(ProductionSite, productionsite, get_location(game))){
+					if (!found_reserved) {
+						productionsite->notify_player(game, 30);
+					}
+					else {
+						productionsite->unnotify_player();
+					}
+				}
 				return true;
 			}
+			//Todo(DaAlx1): Check whether this else branch is a copy and paste error?
 			else {
 				if (upcast(ProductionSite, productionsite, get_location(game)))
 					productionsite->unnotify_player();
@@ -459,19 +467,27 @@
 			std::vector<Bob *> list;
 			if (action.iparam2 < 0)
 				map.find_reachable_bobs
-					(area, &list, cstep);
+						(area, &list, cstep);
 			else
 				map.find_reachable_bobs
-					(area, &list, cstep, FindBobAttribute(action.iparam2));
+						(area, &list, cstep, FindBobAttribute(action.iparam2));
 
+			for (int idx = list.size() - 1; idx >= 0; idx--) {
+				if (upcast(MapObject, bop, list[idx])) {
+					if (bop->is_reserved_by_worker()) {
+						found_reserved = true;
+						list.erase(list.begin() + idx);
+					}
+				}
+			}
 			if (!list.empty()) {
 				set_program_objvar
-					(game, state, list[game.logic_rand() % list.size()]);
+						(game, state, list[game.logic_rand() % list.size()]);
 				break;
 			}
+
 		}
 	}
-
 	++state.ivar1;
 	schedule_act(game, 10);
 	return true;
@@ -565,7 +581,7 @@
 		if (action.iparam4)
 			functor.add
 				(FindNodeResourceBreedable
-				 	(world.get_resource(action.sparam1.c_str())));
+					(world.get_resource(action.sparam1.c_str())));
 		else
 			functor.add
 				(FindNodeResource(world.get_resource(action.sparam1.c_str())));
@@ -662,11 +678,11 @@
 	if
 		(!
 		 start_task_movepath
-		 	(game,
-		 	 dest,
-		 	 10,
-		 	 descr().get_right_walk_anims(does_carry_ware()),
-		 	 forceonlast, max_steps))
+			(game,
+			 dest,
+			 10,
+			 descr().get_right_walk_anims(does_carry_ware()),
+			 forceonlast, max_steps))
 	{
 		molog("  could not find path\n");
 		send_signal(game, "fail");
@@ -945,7 +961,7 @@
 		//NoLog("  Field is no longer empty\n");
 	} else if
 		(const ResourceDescription * const rdescr =
-		 	world.get_resource(position.field->get_resources()))
+			world.get_resource(position.field->get_resources()))
 	{
 		// Geologist also sends a message notifying the player
 		if (rdescr->detectable() && position.field->get_resources_amount()) {
@@ -976,9 +992,9 @@
 				 *new Message
 					(message_type,
 					 game.get_gametime(),
-				 	 rdescr->descname(),
-				 	 message,
-				 	 position,
+					 rdescr->descname(),
+					 message,
+					 position,
 					 m_serial
 					),
 				 300000, 8);
@@ -988,9 +1004,9 @@
 		game.create_immovable
 			(position,
 			 t.get_resource_indicator
-			 	(rdescr,
-			 	 rdescr->detectable() ?
-			 	 position.field->get_resources_amount() : 0),
+				(rdescr,
+				 rdescr->detectable() ?
+				 position.field->get_resources_amount() : 0),
 			 &t);
 	}
 
@@ -1314,8 +1330,8 @@
  */
 WareIndex Worker::gain_experience(Game & game) {
 	return descr().get_needed_experience() == -1 || ++m_current_exp < descr().get_needed_experience() ?
-	          INVALID_INDEX :
-	          level(game);
+			  INVALID_INDEX :
+			  level(game);
 }
 
 
@@ -1548,10 +1564,10 @@
 			if (index >= 0) {
 				if
 					(start_task_movepath
-					 	(game,
-					 	 path,
-					 	 index,
-					 	 descr().get_right_walk_anims(does_carry_ware())))
+						(game,
+						 path,
+						 index,
+						 descr().get_right_walk_anims(does_carry_ware())))
 				{
 					molog
 						("[transfer]: from road %u to flag %u\n",
@@ -1861,10 +1877,10 @@
 	if
 		(!
 		 start_task_movepath
-		 	(game,
-		 	 location->base_flag().get_position(),
-		 	 15,
-		 	 descr().get_right_walk_anims(does_carry_ware())))
+			(game,
+			 location->base_flag().get_position(),
+			 15,
+			 descr().get_right_walk_anims(does_carry_ware())))
 	{
 		molog("[return]: Failed to return\n");
 		const std::string message =
@@ -1876,9 +1892,9 @@
 			 *new Message
 				(Message::Type::kGameLogic,
 				 game.get_gametime(),
-			 	 _("Worker got lost!"),
+				 _("Worker got lost!"),
 				 message,
-			 	 get_position()),
+				 get_position()),
 				 m_serial);
 		set_location(nullptr);
 		return pop_task(game);
@@ -1951,14 +1967,15 @@
 {
 	assert(state.task == &taskProgram);
 
-	if (upcast(Immovable, imm, state.objvar1.get(game))) {
-		imm->set_reserved_by_worker(false);
+
+	if (state.objvar1.get(game)) {
+		(state.objvar1.get(game))->set_reserved_by_worker(false);
 	}
 
 	state.objvar1 = obj;
 
-	if (upcast(Immovable, imm, obj)) {
-		imm->set_reserved_by_worker(true);
+	if (obj) {
+		obj->set_reserved_by_worker(true);
 	}
 }
 
@@ -2558,8 +2575,8 @@
 	int32_t maxdist = 4 * vision;
 	if
 		(map.find_immovables
-		 	(Area<FCoords>(map.get_fcoords(get_position()), maxdist),
-		 	 &flags, FindFlagWithPlayersWarehouse(*get_owner())))
+			(Area<FCoords>(map.get_fcoords(get_position()), maxdist),
+			 &flags, FindFlagWithPlayersWarehouse(*get_owner())))
 	{
 		int32_t bestdist = -1;
 		Flag *  best     =  nullptr;
@@ -2596,12 +2613,12 @@
 			// perhaps find a closer flag.
 			if
 				(start_task_movepath
-				 	(game,
-				 	 best->get_position(),
-				 	 0,
-				 	 descr().get_right_walk_anims(does_carry_ware()),
-				 	 false,
-				 	 4))
+					(game,
+					 best->get_position(),
+					 0,
+					 descr().get_right_walk_anims(does_carry_ware()),
+					 false,
+					 4))
 				return;
 		}
 	}
@@ -2615,10 +2632,10 @@
 
 	if
 		(start_task_movepath
-		 	(game,
-		 	 game.random_location(get_position(), descr().vision_range()),
-		 	 4,
-		 	 descr().get_right_walk_anims(does_carry_ware())))
+			(game,
+			 game.random_location(get_position(), descr().vision_range()),
+			 4,
+			 descr().get_right_walk_anims(does_carry_ware())))
 		return;
 
 	return start_task_idle(game, descr().get_animation("idle"), 50);
@@ -2674,7 +2691,7 @@
 	const World & world = game.world();
 	Area<FCoords> owner_area
 		(map.get_fcoords
-		 	(dynamic_cast<Flag&>(*get_location(game)).get_position()),
+			(dynamic_cast<Flag&>(*get_location(game)).get_position()),
 		 state.ivar2);
 
 	// Check if it's not time to go home
@@ -2744,10 +2761,10 @@
 				if
 					(!
 					 start_task_movepath
-					 	(game,
-					 	 target,
-					 	 0,
-					 	 descr().get_right_walk_anims(does_carry_ware())))
+						(game,
+						 target,
+						 0,
+						 descr().get_right_walk_anims(does_carry_ware())))
 				{
 
 					molog("[geologist]: Bug: could not find path\n");
@@ -2767,7 +2784,7 @@
 	if
 		(!
 		 start_task_movepath
-		 	(game, owner_area, 0, descr().get_right_walk_anims(does_carry_ware())))
+			(game, owner_area, 0, descr().get_right_walk_anims(does_carry_ware())))
 	{
 		molog("[geologist]: could not find path home\n");
 		send_signal(game, "fail");