From ab1c598d4e7fd4e89184b0c5d55ff418db8f7d27 Mon Sep 17 00:00:00 2001
From: Ewilhan <45350161+Ewilhan@users.noreply.github.com>
Date: Thu, 16 May 2019 14:41:02 +0100
Subject: [PATCH 1/3] Fixed Battle Creature Order

Fixed the messy creature order. It now follows correctly all creature ordering rules.
---
 lib/battle/BattleInfo.cpp          |  29 ++++---
 lib/battle/BattleInfo.h            |   3 +-
 lib/battle/CBattleInfoCallback.cpp | 127 +++++++++++++++++------------
 3 files changed, 94 insertions(+), 65 deletions(-)

diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp
index c2e6a5cce..d30bec08e 100644
--- a/lib/battle/BattleInfo.cpp
+++ b/lib/battle/BattleInfo.cpp
@@ -804,7 +804,6 @@ void BattleInfo::addUnit(uint32_t id, const JsonNode & data)
 void BattleInfo::moveUnit(uint32_t id, BattleHex destination)
 {
 	auto sta = getStack(id);
-
 	if(!sta)
 	{
 		logGlobal->error("Cannot find stack %d", id);
@@ -1063,28 +1062,38 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 			int as = a->getInitiative(turn), bs = b->getInitiative(turn);
 			if(as != bs)
 				return as > bs;
-			else
-				return a->unitSlot() < b->unitSlot(); //FIXME: what about summoned stacks?
-		}
+			else if(as == bs)
+			{
+				if(a->unitSide() == b->unitSide())
+					return a->unitSlot() < b->unitSlot();
+				else
+					return a->unitSide() == side ? false : true;
+			}
+			//FIXME: what about summoned stacks
+		}	
 	case 2: //fastest last, upper slot first
-		//TODO: should be replaced with order of receiving morale!
 	case 3: //fastest last, upper slot first
 		{
 			int as = a->getInitiative(turn), bs = b->getInitiative(turn);
 			if(as != bs)
-				return as < bs;
-			else
-				return a->unitSlot() < b->unitSlot();
+				return as > bs;
+			else if(as == bs)
+			{
+				if(a->unitSide() == b->unitSide())
+					return a->unitSlot() < b->unitSlot();
+				else
+					return a->unitSide() == side ? false : true;
+			}
 		}
 	default:
 		assert(0);
 		return false;
 	}
-
 }
 
-CMP_stack::CMP_stack(int Phase, int Turn)
+CMP_stack::CMP_stack(int Phase, int Turn, int Side)
 {
 	phase = Phase;
 	turn = Turn;
+	side = Side;
 }
diff --git a/lib/battle/BattleInfo.h b/lib/battle/BattleInfo.h
index a7bc51341..9e5d3b9c6 100644
--- a/lib/battle/BattleInfo.h
+++ b/lib/battle/BattleInfo.h
@@ -144,8 +144,9 @@ class DLL_LINKAGE CMP_stack
 {
 	int phase; //rules of which phase will be used
 	int turn;
+	bool side;
 public:
 
 	bool operator ()(const battle::Unit * a, const battle::Unit * b);
-	CMP_stack(int Phase = 1, int Turn = 0);
+	CMP_stack(int Phase = 1, int Turn = 0, int Side = BattleSide::ATTACKER);
 };
diff --git a/lib/battle/CBattleInfoCallback.cpp b/lib/battle/CBattleInfoCallback.cpp
index 698d3cf49..3adf7e90e 100644
--- a/lib/battle/CBattleInfoCallback.cpp
+++ b/lib/battle/CBattleInfoCallback.cpp
@@ -353,57 +353,67 @@ battle::Units CBattleInfoCallback::battleAliveUnits(ui8 side) const
 
 //T is battle::Unit descendant
 template <typename T>
-const T * takeOneUnit(std::vector<const T *> & all, const int turn, int8_t & lastMoved)
+const T * takeOneUnit(std::vector<const T*> & all, const int turn, int8_t & lastMoved, int phase)
 {
-	const T * ret = nullptr;
-	size_t i, //fastest stack
-			j=0; //fastest stack of the other side
-	for(i = 0; i < all.size(); i++)
+	const T * retCreature = nullptr;
+	size_t fastestIndex = 0;
+
+	for(int i = 0; i < all.size(); i++)
+	{
+		int32_t curUnitSpeed = -1;
+		int32_t retUnitSpeed = -1;
 		if(all[i])
+			curUnitSpeed = all[i]->getInitiative(turn);
+		if(retCreature)
+			retUnitSpeed = retCreature->getInitiative(turn);
+		
+		switch(phase)
+		{
+		case 1: // Faster first, attacker priority, higher slot first
+			if(all[i] && (retCreature == nullptr || (curUnitSpeed > retUnitSpeed)))
+			{
+				retCreature = all[i];
+				fastestIndex = i;
+			} 
+			else if(all[i] && (curUnitSpeed == retUnitSpeed))
+			{
+				if(lastMoved == -1 && turn <= 0 && all[i]->unitSide() == BattleSide::ATTACKER
+					&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Turn 0 attacker priority
+				{
+					retCreature = all[i];
+					fastestIndex = i;
+				}
+				else if(lastMoved != -1 && all[i]->unitSide() != lastMoved
+						&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
+				{
+					retCreature = all[i];
+					fastestIndex = i;
+				}
+			}
 			break;
+		case 2: // Slower first, higher slot first
+		case 3:
+			if(all[i] && (retCreature == nullptr || (curUnitSpeed < retUnitSpeed)))
+			{
+				retCreature = all[i];
+				fastestIndex = i;
+			}
+			else if(all[i] && curUnitSpeed == retUnitSpeed && lastMoved != -1 && all[i]->unitSide() != lastMoved
+					&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
+			{
+				retCreature = all[i];
+				fastestIndex = i;
+			}
+			break;
+		default:
+			break;
+		}
+	}
 
-	//no stacks left
-	if(i == all.size())
+	if(!retCreature)
 		return nullptr;
-
-	const T * fastest = all[i], *other = nullptr;
-	int bestSpeed = fastest->getInitiative(turn);
-
-	if(fastest->unitSide() == lastMoved)
-	{
-		ret = fastest;
-	}
-	else
-	{
-		for(j = i + 1; j < all.size(); j++)
-		{
-			if(!all[j]) continue;
-			if(all[j]->unitSide() != lastMoved || all[j]->getInitiative(turn) != bestSpeed)
-				break;
-		}
-
-		if(j >= all.size())
-		{
-			ret = fastest;
-		}
-		else
-		{
-			other = all[j];
-			if(other->getInitiative(turn) != bestSpeed)
-				ret = fastest;
-			else
-				ret = other;
-		}
-	}
-
-	assert(ret);
-	if(ret == fastest)
-		all[i] = nullptr;
-	else
-		all[j] = nullptr;
-
-	lastMoved = ret->unitSide();
-	return ret;
+	all[fastestIndex] = nullptr;
+	return retCreature;
 }
 
 void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, const size_t maxUnits, const int maxTurns, const int turn, int8_t lastMoved) const
@@ -483,28 +493,37 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, c
 		phase[p].push_back(one);
 	}
 
-	boost::sort(phase[0], CMP_stack(0, actualTurn));
+	boost::sort(phase[0], CMP_stack(0, actualTurn, lastMoved));
 	std::copy(phase[0].begin(), phase[0].end(), std::back_inserter(out.back()));
 
 	if(outputFull())
 		return;
 
 	for(int i = 1; i < 4; i++)
-		boost::sort(phase[i], CMP_stack(i, actualTurn));
-
-	if(lastMoved < 0)
-		lastMoved = BattleSide::ATTACKER;
+		boost::sort(phase[i], CMP_stack(i, actualTurn, lastMoved));	
 
 	int pi = 1;
 	while(!outputFull() && pi < 4)
 	{
-		auto current = takeOneUnit(phase[pi], actualTurn, lastMoved);
-		if(!current)
+		const battle::Unit * current = nullptr;
+		if(phase[pi].empty())
 			pi++;
 		else
-			out.back().push_back(current);
+		{
+			current = takeOneUnit(phase[pi], actualTurn, lastMoved, pi);
+			if(!current)
+				pi++;
+			else
+			{
+				out.back().push_back(current);
+				lastMoved = current->unitSide();	
+			}
+		}
 	}
 
+	if (lastMoved < 0)
+		lastMoved = BattleSide::ATTACKER;
+
 	if(!outputFull() && (maxTurns == 0 || out.size() < maxTurns))
 		battleGetTurnOrder(out, maxUnits, maxTurns, actualTurn + 1, lastMoved);
 }

From 023d6c88fd479437c0a71cbfb8e0bc05a50ca6cc Mon Sep 17 00:00:00 2001
From: Ewilhan <45350161+Ewilhan@users.noreply.github.com>
Date: Fri, 17 May 2019 09:34:59 +0100
Subject: [PATCH 2/3] Code style and improved readability

---
 lib/battle/BattleInfo.cpp          |  6 +-
 lib/battle/BattleInfo.h            |  4 +-
 lib/battle/CBattleInfoCallback.cpp | 91 +++++++++++++++---------------
 3 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp
index d30bec08e..62e8e0c68 100644
--- a/lib/battle/BattleInfo.cpp
+++ b/lib/battle/BattleInfo.cpp
@@ -1062,7 +1062,7 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 			int as = a->getInitiative(turn), bs = b->getInitiative(turn);
 			if(as != bs)
 				return as > bs;
-			else if(as == bs)
+			else
 			{
 				if(a->unitSide() == b->unitSide())
 					return a->unitSlot() < b->unitSlot();
@@ -1077,7 +1077,7 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 			int as = a->getInitiative(turn), bs = b->getInitiative(turn);
 			if(as != bs)
 				return as > bs;
-			else if(as == bs)
+			else
 			{
 				if(a->unitSide() == b->unitSide())
 					return a->unitSlot() < b->unitSlot();
@@ -1091,7 +1091,7 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 	}
 }
 
-CMP_stack::CMP_stack(int Phase, int Turn, int Side)
+CMP_stack::CMP_stack(int Phase, int Turn, uint8_t Side)
 {
 	phase = Phase;
 	turn = Turn;
diff --git a/lib/battle/BattleInfo.h b/lib/battle/BattleInfo.h
index 9e5d3b9c6..b66acb701 100644
--- a/lib/battle/BattleInfo.h
+++ b/lib/battle/BattleInfo.h
@@ -144,9 +144,9 @@ class DLL_LINKAGE CMP_stack
 {
 	int phase; //rules of which phase will be used
 	int turn;
-	bool side;
+	uint8_t side;
 public:
 
 	bool operator ()(const battle::Unit * a, const battle::Unit * b);
-	CMP_stack(int Phase = 1, int Turn = 0, int Side = BattleSide::ATTACKER);
+	CMP_stack(int Phase = 1, int Turn = 0, uint8_t Side = BattleSide::ATTACKER);
 };
diff --git a/lib/battle/CBattleInfoCallback.cpp b/lib/battle/CBattleInfoCallback.cpp
index 3adf7e90e..c0f8351ae 100644
--- a/lib/battle/CBattleInfoCallback.cpp
+++ b/lib/battle/CBattleInfoCallback.cpp
@@ -355,65 +355,66 @@ battle::Units CBattleInfoCallback::battleAliveUnits(ui8 side) const
 template <typename T>
 const T * takeOneUnit(std::vector<const T*> & all, const int turn, int8_t & lastMoved, int phase)
 {
-	const T * retCreature = nullptr;
-	size_t fastestIndex = 0;
+	const T * returnedUnit = nullptr;
+	size_t currentUnitIndex = 0;
 
-	for(int i = 0; i < all.size(); i++)
+	for(size_t i = 0; i < all.size(); i++)
 	{
-		int32_t curUnitSpeed = -1;
-		int32_t retUnitSpeed = -1;
+		int32_t currentUnitSpeed = -1;
+		int32_t returnedUnitSpeed = -1;
+		if(returnedUnit)
+			returnedUnitSpeed = returnedUnit->getInitiative(turn);
 		if(all[i])
-			curUnitSpeed = all[i]->getInitiative(turn);
-		if(retCreature)
-			retUnitSpeed = retCreature->getInitiative(turn);
-		
-		switch(phase)
 		{
-		case 1: // Faster first, attacker priority, higher slot first
-			if(all[i] && (retCreature == nullptr || (curUnitSpeed > retUnitSpeed)))
+			currentUnitSpeed = all[i]->getInitiative(turn);
+			switch (phase)
 			{
-				retCreature = all[i];
-				fastestIndex = i;
-			} 
-			else if(all[i] && (curUnitSpeed == retUnitSpeed))
-			{
-				if(lastMoved == -1 && turn <= 0 && all[i]->unitSide() == BattleSide::ATTACKER
-					&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Turn 0 attacker priority
+			case 1: // Faster first, attacker priority, higher slot first
+				if(returnedUnit == nullptr || currentUnitSpeed > returnedUnitSpeed)
 				{
-					retCreature = all[i];
-					fastestIndex = i;
+					returnedUnit = all[i];
+					currentUnitIndex = i;
 				}
-				else if(lastMoved != -1 && all[i]->unitSide() != lastMoved
-						&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
+				else if(currentUnitSpeed == returnedUnitSpeed)
 				{
-					retCreature = all[i];
-					fastestIndex = i;
+					if(lastMoved == -1 && turn <= 0 && all[i]->unitSide() == BattleSide::ATTACKER
+						&& !(returnedUnit->unitSide() == all[i]->unitSide() && returnedUnit->unitSlot() < all[i]->unitSlot())) // Turn 0 attacker priority
+					{
+						returnedUnit = all[i];
+						currentUnitIndex = i;
+					}
+					else if(lastMoved != -1 && all[i]->unitSide() != lastMoved
+						&& !(returnedUnit->unitSide() == all[i]->unitSide() && returnedUnit->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
+					{
+						returnedUnit = all[i];
+						currentUnitIndex = i;
+					}
 				}
+				break;
+			case 2: // Slower first, higher slot first
+			case 3:
+				if(returnedUnit == nullptr || currentUnitSpeed < returnedUnitSpeed)
+				{
+					returnedUnit = all[i];
+					currentUnitIndex = i;
+				}
+				else if(currentUnitSpeed == returnedUnitSpeed && lastMoved != -1 && all[i]->unitSide() != lastMoved
+					&& !(returnedUnit->unitSide() == all[i]->unitSide() && returnedUnit->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
+				{
+					returnedUnit = all[i];
+					currentUnitIndex = i;
+				}
+				break;
+			default:
+				break;
 			}
-			break;
-		case 2: // Slower first, higher slot first
-		case 3:
-			if(all[i] && (retCreature == nullptr || (curUnitSpeed < retUnitSpeed)))
-			{
-				retCreature = all[i];
-				fastestIndex = i;
-			}
-			else if(all[i] && curUnitSpeed == retUnitSpeed && lastMoved != -1 && all[i]->unitSide() != lastMoved
-					&& !(retCreature->unitSide() == all[i]->unitSide() && retCreature->unitSlot() < all[i]->unitSlot())) // Alternate equal speeds units
-			{
-				retCreature = all[i];
-				fastestIndex = i;
-			}
-			break;
-		default:
-			break;
 		}
 	}
 
-	if(!retCreature)
+	if(!returnedUnit)
 		return nullptr;
-	all[fastestIndex] = nullptr;
-	return retCreature;
+	all[currentUnitIndex] = nullptr;
+	return returnedUnit;
 }
 
 void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, const size_t maxUnits, const int maxTurns, const int turn, int8_t lastMoved) const

From cba94f8bb3a4ae23debe36595b2440fb4572a627 Mon Sep 17 00:00:00 2001
From: Ewilhan <45350161+Ewilhan@users.noreply.github.com>
Date: Sat, 18 May 2019 18:37:02 +0200
Subject: [PATCH 3/3] Code style

Co-Authored-By: Alexander Shishkin <alexvins@users.noreply.github.com>
---
 lib/battle/CBattleInfoCallback.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/battle/CBattleInfoCallback.cpp b/lib/battle/CBattleInfoCallback.cpp
index c0f8351ae..fb305b16b 100644
--- a/lib/battle/CBattleInfoCallback.cpp
+++ b/lib/battle/CBattleInfoCallback.cpp
@@ -367,7 +367,7 @@ const T * takeOneUnit(std::vector<const T*> & all, const int turn, int8_t & last
 		if(all[i])
 		{
 			currentUnitSpeed = all[i]->getInitiative(turn);
-			switch (phase)
+			switch(phase)
 			{
 			case 1: // Faster first, attacker priority, higher slot first
 				if(returnedUnit == nullptr || currentUnitSpeed > returnedUnitSpeed)
@@ -513,7 +513,9 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, c
 		{
 			current = takeOneUnit(phase[pi], actualTurn, lastMoved, pi);
 			if(!current)
+			{
 				pi++;
+			}
 			else
 			{
 				out.back().push_back(current);
@@ -522,7 +524,7 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, c
 		}
 	}
 
-	if (lastMoved < 0)
+	if(lastMoved < 0)
 		lastMoved = BattleSide::ATTACKER;
 
 	if(!outputFull() && (maxTurns == 0 || out.size() < maxTurns))