Merge lp:~nha/widelands/bug1132473 into lp:widelands

[Nicolai Hähnle]

Fix bug #1132473 and other cleanups in the soldier code

Attacking soldiers no longer get stuck when their location disappears while they are in battle. Furthermore, replace some polling in the battle code by a signal mechanism to avoid unnecessary work, and remove some particularly spammy log messages.

[SirVer (sirver)]

Late to the party - as always lately :/. LGTM, I think all changes improve the code.

review: Approve

=== modified file 'src/logic/bob.cc'
--- src/logic/bob.cc	2013-03-03 19:46:00 +0000
+++ src/logic/bob.cc	2013-03-16 12:44:24 +0000
@@ -1040,7 +1040,7 @@
 		molog("* svar1: %s\n", m_stack[i].svar1.c_str());
 
 		molog("* coords: (%i, %i)\n", m_stack[i].coords.x, m_stack[i].coords.y);
-		molog("* diranims:",  m_stack[i].diranims);
+		molog("* diranims:");
 		for (Direction dir = FIRST_DIRECTION; dir <= LAST_DIRECTION; ++dir) {
 			molog(" %d", m_stack[i].diranims.get_animation(dir));
 		}

=== modified file 'src/logic/soldier.cc'
--- src/logic/soldier.cc	2013-03-03 19:46:00 +0000
+++ src/logic/soldier.cc	2013-03-16 12:44:24 +0000
@@ -292,7 +292,6 @@
 		run = m_die_e_name[i];
 	}
 
-	log(" get %s\n", run.c_str());
 	return get_animation(run.c_str());
 }
 
@@ -832,11 +831,10 @@
 		return start_task_idle(game, get_animation("idle"), -1);
 	}
 
-	PlayerImmovable * const location = get_location(game);
-	BaseImmovable * const imm = game.map()[get_position()].get_immovable();
+	upcast(Building, location, get_location(game));
 	upcast(Building, enemy, state.objvar1.get(game));
 
-	if (imm == location) {
+	if (location && get_position() == location->get_position()) {
 		if (!enemy) {
 			molog("[attack] returned home\n");
 			return pop_task(game);
@@ -847,12 +845,13 @@
 	if (m_battle)
 		return start_task_battle(game);
 
-	if (signal == "blocked")
+	if (signal == "blocked") {
 		// Wait before we try again. Note that this must come *after*
 		// we check for a battle
 		// Note that we *should* be woken via sendSpaceSignals,
 		// so the timeout is just an additional safety net.
 		return start_task_idle(game, get_animation("idle"), 5000);
+	}
 
 	if (!location) {
 		molog("[attack] our location disappeared during a battle\n");
@@ -915,7 +914,7 @@
 			}
 		}
 		Flag & baseflag = location->base_flag();
-		if (imm == &baseflag)
+		if (get_position() == baseflag.get_position())
 			return
 				start_task_move
 					(game,
@@ -938,7 +937,7 @@
 
 	// At this point, we know that the enemy building still stands,
 	// and that we're outside in the plains.
-	if (imm != &enemy->base_flag()) {
+	if (get_position() != enemy->base_flag().get_position()) {
 		if
 			(start_task_movepath
 			 	(game,
@@ -1414,6 +1413,7 @@
 			return skip_act(); //  we will get a signal via setBattle()
 		} else {
 			if (m_combat_walking != CD_COMBAT_E) {
+				opponent.send_signal(game, "wakeup");
 				return start_task_move_in_battle(game, CD_WALK_E);
 			}
 		}
@@ -1495,7 +1495,6 @@
 				}
 			}
 		} else {
-
 			assert(opponent.get_position() == get_position());
 			assert(m_battle == opponent.getBattle());
 
@@ -1503,22 +1502,21 @@
 				molog
 					("[battle]: Opponent '%d' is walking, sleeping\n",
 					 opponent.serial());
-				return start_task_idle(game, descr().get_animation("idle"), 100);
+				// We should be woken up by our opponent, but add a timeout anyway for robustness
+				return start_task_idle(game, descr().get_animation("idle"), 5000);
 			}
 
 			if (m_battle->first()->serial() == serial()) {
-				molog("[battle]: I am first: '%d'\n", m_combat_walking);
 				if (m_combat_walking != CD_COMBAT_W) {
-					start_task_move_in_battle(game, CD_WALK_W);
-					return;
+					molog("[battle]: Moving west\n");
+					opponent.send_signal(game, "wakeup");
+					return start_task_move_in_battle(game, CD_WALK_W);
 				}
-			}
-
-			if (m_battle->second()->serial() == serial()) {
-				molog("[battle]: I am second: '%d'\n", m_combat_walking);
+			} else {
 				if (m_combat_walking != CD_COMBAT_E) {
-					start_task_move_in_battle(game, CD_WALK_E);
-					return;
+					molog("[battle]: Moving east\n");
+					opponent.send_signal(game, "wakeup");
+					return start_task_move_in_battle(game, CD_WALK_E);
 				}
 			}
 		}