widelands-dev team mailing list archive

[Nicolai Hähnle <prefect_@xxxxxxx>]

Nicolai Hähnle has proposed merging lp:~nha/widelands/bug1132473 into lp:widelands.

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

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1132473 in widelands: "soldier hangs at one point"
  https://bugs.launchpad.net/widelands/+bug/1132473

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

=== 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);
 				}
 			}
 		}