widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

TiborB has proposed merging lp:~widelands-dev/widelands/bug-1457425 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1457425 in widelands: "Fight never end (also not 2h later)"
  https://bugs.launchpad.net/widelands/+bug/1457425

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1457425/+merge/263620

This is intended as a solution of situation when battle is initiated (soldiers are paired) but one of them is blocked by other soldiers (or other obstacles) and battle lasts forever (fight never starts). It introduces fourth status "waitingForOpponent" - this is basically status when first three statuses is false. Only purpose of this status is test lasted time since the battle object was created and if above threshold (90 sec now), the battle is cancelled. Based on my testing, I can say such cancellation is extremely rare, probably only when the game is stuck permanently.
But somebody who is familiar with soldier/battle code should look at the design. It works, but I am not sure how proper it is. To test it, you can reduce the timeout to like 5 second (to prove the game does not crash on cancelling the battle)
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1457425 into lp:widelands.

=== modified file 'src/logic/battle.cc'
--- src/logic/battle.cc	2014-11-30 18:49:38 +0000
+++ src/logic/battle.cc	2015-07-02 06:55:44 +0000
@@ -157,17 +157,20 @@
 
 	assert(m_first->get_battle() == this || m_second->get_battle() == this);
 
-	//  Created this three 'states' of the battle:
+	//  Created this four 'states' of the battle:
 	// *First time entered, one enters :
 	//    oneReadyToFight, mark m_readyflags as he is ready to fight
 	// *Next time, the opponent enters:
 	//    bothReadyToFight, mark m_readyflags as 3 (round fighted)
 	// *Next time, the first enters again:
 	//    roundFighted, reset m_readyflags
+	// *Opponent not on field yet, so one enters :
+	//    waitingForOpponent, if others are false
 	bool const oneReadyToFight  = (m_readyflags == 0);
 	bool const roundFighted     = (m_readyflags == 3);
 	bool const bothReadyToFight = ((this_soldier_is | m_readyflags) == 3) &&
 		(!roundFighted);
+	bool const waitingForOpponent = !(oneReadyToFight || roundFighted || bothReadyToFight);
 	std::string what_anim;
 
 	// Apply pending damage
@@ -193,6 +196,16 @@
 	if (!m_first || !m_second)
 		return soldier.skip_act();
 
+	//Here is a timeout to prevent battle freezes
+	if (waitingForOpponent && (game.get_gametime() - m_creationtime) > 90 * 1000) {
+		molog("[battle] soldier %u waiting for opponent %u too long (%5d sec), cancelling battle...\n",
+			soldier.serial(),
+			opponent(soldier)->serial(),
+			(game.get_gametime() - m_creationtime) / 1000);
+		cancel(game, soldier);
+		return;
+	}
+
 	// So both soldiers are alive; are we ready to trade the next blow?
 	//
 	//  This code choses one of 3 codepaths: