widelands-dev team mailing list archive

[Nicolai Hähnle <prefect_@xxxxxxx>]

Nicolai Hähnle has proposed merging lp:~widelands-dev/widelands/bug1132466 into lp:widelands.

Commit message:
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.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1132466 in widelands: "Evicted workers will become stuck if the are away from home and the building is not conencted to the road network"
  https://bugs.launchpad.net/widelands/+bug/1132466

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

=== modified file 'src/logic/playercommand.cc'
--- src/logic/playercommand.cc	2013-02-04 21:52:38 +0000
+++ src/logic/playercommand.cc	2013-02-24 21:30:28 +0000
@@ -638,8 +638,14 @@
 {
 	upcast(Worker, worker, game.objects().get_object(serial));
 	if (worker && worker->owner().player_number() == sender()) {
-		worker->reset_tasks(game);
-		worker->start_task_gowarehouse(game);
+		if (upcast(Building, building, worker->get_location(game))) {
+			if (upcast(Soldier, soldier, worker)) {
+				if (soldier->get_position() != building->get_position())
+					return;
+			}
+			worker->reset_tasks(game);
+			worker->start_task_gowarehouse(game);
+		}
 	}
 }
 

=== modified file 'src/logic/worker.cc'
--- src/logic/worker.cc	2013-02-11 18:01:26 +0000
+++ src/logic/worker.cc	2013-02-24 21:30:28 +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);
 		}
@@ -1998,9 +1988,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 +2372,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 +2392,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;
+	}
 }