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

[Nicolai Hähnle]

Fix bug #1132466 and a potential cheat related to evicting soldiers

The worker leavebuilding task now deals more robustly with situations in which the evicted worker is currently outside of the building.

Furthermore, it is no longer possible to evict soldiers whose current position on the map is not their assigned building (i.e., their location). Note that this was not possible with the standard user interface anyway, but a cheater could have modified her version of the game such that an eviction player command would be sent automatically for her soldiers when they reach low HP during a battle. Such soldiers would have immediately stopped battle and returned home.

[SirVer (sirver)]

lgtm.

review: Approve

[Nicolai Hähnle (nha)]

Thank you. However, I just realized myself that the eviction still has a problem that I would like to fix first: When evicting a worker that is in the middle of walking, the reset_tasks() approach that is being taken right now causes the worker to be "warped".

[Nicolai Hähnle (nha)]

This fixes the warping issue, and seems to be the correct of doing it.

[SirVer (sirver)]

+ const Coords flagpos = baseflag.get_position();
can this be a const reference? Haven't checked.

still lgtm.

review: Approve

=== modified file 'src/logic/playercommand.cc'
--- src/logic/playercommand.cc	2013-02-04 21:52:38 +0000
+++ src/logic/playercommand.cc	2013-02-25 17:40:30 +0000
@@ -638,8 +638,7 @@
 {
 	upcast(Worker, worker, game.objects().get_object(serial));
 	if (worker && worker->owner().player_number() == sender()) {
-		worker->reset_tasks(game);
-		worker->start_task_gowarehouse(game);
+		worker->evict(game);
 	}
 }
 

=== modified file 'src/logic/soldier.cc'
--- src/logic/soldier.cc	2013-02-10 19:36:24 +0000
+++ src/logic/soldier.cc	2013-02-25 17:40:30 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -347,6 +347,11 @@
 	Worker::cleanup(egbase);
 }
 
+bool Soldier::is_evict_allowed()
+{
+	return !isOnBattlefield();
+}
+
 /*
  * Set this soldiers level. Automatically sets the new values
  */

=== modified file 'src/logic/soldier.h'
--- src/logic/soldier.h	2013-02-10 19:36:24 +0000
+++ src/logic/soldier.h	2013-02-25 17:40:30 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2010 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -287,6 +287,8 @@
 	// May be this can be moved this to bob when finished
 	static Task const taskDie;
 
+	virtual bool is_evict_allowed();
+
 private:
 	uint32_t m_hp_current;
 	uint32_t m_hp_level;

=== modified file 'src/logic/worker.cc'
--- src/logic/worker.cc	2013-02-11 18:01:26 +0000
+++ src/logic/worker.cc	2013-02-25 17:40:30 +0000
@@ -1441,30 +1441,20 @@
 		}
 	}
 
-	// If our location is a building, make sure we're actually in it.
-	// If we're a building's worker, and we've just been released from
-	// the building, we may be somewhere else entirely (e.g. lumberjack, soldier)
-	// or we may be on the building's flag for a fetch_from_flag or dropoff
-	// task.
+	// If our location is a building, our position may be somewhere else:
+	// We may be on the building's flag for a fetch_from_flag or dropoff task.
+	// We may also be somewhere else entirely (e.g. lumberjack, soldier).
 	// Similarly for flags.
-	if (dynamic_cast<Building const *>(location)) {
-		BaseImmovable * const position = map[get_position()].get_immovable();
-
-		if (position != location) {
-			if (upcast(Flag, flag, position)) {
-				location = flag;
-				set_location(flag);
-			} else
-				return set_location(0);
-		}
+	if (upcast(Building, building, location)) {
+		if (building->get_position() != get_position())
+			return start_task_leavebuilding(game, true);
 	} else if (upcast(Flag, flag, location)) {
 		BaseImmovable * const position = map[get_position()].get_immovable();
 
 		if (position != flag) {
 			if (position == flag->get_building()) {
-				upcast(Building, building, position);
-				set_location(building);
-				location = building;
+				location = flag->get_building();
+				set_location(location);
 			} else
 				return set_location(0);
 		}
@@ -1681,6 +1671,7 @@
  *
  * ivar1 - 0: no task has failed; 1: currently in buildingwork;
  *         2: signal failure of buildingwork
+ * ivar2 - whether the worker is to be evicted
  */
 const Bob::Task Worker::taskBuildingwork = {
 	"buildingwork",
@@ -1697,7 +1688,9 @@
 void Worker::start_task_buildingwork(Game & game)
 {
 	push_task(game, taskBuildingwork);
-	top_state().ivar1 = 0;
+	State & state = top_state();
+	state.ivar1 = 0;
+	state.ivar2 = 0;
 }
 
 
@@ -1710,6 +1703,9 @@
 	if (state.ivar1 == 1)
 		state.ivar1 = (signal == "fail") * 2;
 
+	if (state.ivar2 == 1)
+		return pop_task(game); // evict worker
+
 	// Return to building, if necessary
 	upcast(Building, building, get_location(game));
 	if (!building)
@@ -1743,6 +1739,23 @@
 		send_signal(game, "update");
 }
 
+/**
+ * Evict the worker from its current building.
+ */
+void Worker::evict(Game & game)
+{
+	if (State * state = get_state(taskBuildingwork)) {
+		if (is_evict_allowed()) {
+			state->ivar2 = 1;
+			send_signal(game, "evict");
+		}
+	}
+}
+
+bool Worker::is_evict_allowed()
+{
+	return true;
+}
 
 /**
  * Return to our owning building.
@@ -1998,9 +2011,7 @@
 
 	// Always leave buildings in an orderly manner,
 	// even when no warehouses are left to return to
-	if
-		(location->get_type() == BUILDING &&
-		 get_position() == static_cast<Building *>(location)->get_position())
+	if (location->get_type() == BUILDING)
 		return start_task_leavebuilding(game, true);
 
 	if (!get_economy()->warehouses().size()) {
@@ -2384,14 +2395,19 @@
 	else if (signal.size())
 		return pop_task(game);
 
-	if (upcast(Building, building, game.map().get_immovable(get_position()))) {
+	upcast(Building, building, get_location(game));
+	if (!building)
+		return pop_task(game);
+
+	Flag & baseflag = building->base_flag();
+
+	if (get_position() == building->get_position()) {
 		assert(building == state.objvar1.get(game));
-
 		if (!building->leave_check_and_wait(game, *this))
 			return skip_act();
 
 		if (state.ivar1)
-			set_location(&building->base_flag());
+			set_location(&baseflag);
 
 		return
 			start_task_move
@@ -2399,8 +2415,22 @@
 				 WALK_SE,
 				 &descr().get_right_walk_anims(does_carry_ware()),
 				 true);
-	} else
-		return pop_task(game);
+	} else {
+		const Coords flagpos = baseflag.get_position();
+
+		if (state.ivar1)
+			set_location(&baseflag);
+
+		if (get_position() == flagpos)
+			return pop_task(game);
+
+		if (!start_task_movepath(game, flagpos, 0, descr().get_right_walk_anims(does_carry_ware()))) {
+			molog("[leavebuilding]: outside of building, but failed to walk back to flag");
+			set_location(0);
+			return pop_task(game);
+		}
+		return;
+	}
 }
 
 

=== modified file 'src/logic/worker.h'
--- src/logic/worker.h	2013-02-10 19:36:24 +0000
+++ src/logic/worker.h	2013-02-25 17:40:30 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2010 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -171,6 +171,7 @@
 
 	void start_task_buildingwork(Game &);
 	void update_task_buildingwork(Game &);
+	void evict(Game &);
 
 	void start_task_return(Game & game, bool dropitem);
 	void start_task_program(Game & game, const std::string & programname);
@@ -192,6 +193,7 @@
 	void start_task_scout(Game &, uint16_t, uint32_t);
 
 protected:
+	virtual bool is_evict_allowed();
 	void draw_inner(const Editor_Game_Base &, RenderTarget &, Point) const;
 	virtual void draw(const Editor_Game_Base &, RenderTarget &, Point) const;
 	virtual void init_auto_task(Game &);