Minor fixes:

- removed unused functionality from CCreatureAnimation
- simplified postioning of units in battle, should fix remaining issues with unit positioning
- fixed unit tests compilation

client/CAnimation.h

@@ -318,6 +318,15 @@ class CCreatureAnim: public CShowableAnim
 {
 public:
 
+	enum EHeroAnimType
+	{
+		HERO_HOLDING = 0,
+		HERO_IDLE = 1, // idling movement that happens from time to time
+		HERO_DEFEAT = 2, // played when army loses stack or on friendly fire
+		HERO_VICTORY = 3, // when enemy stack killed or huge damage is dealt
+		HERO_CAST_SPELL = 4 // spellcasting
+	};
+
 	enum EAnimType // list of creature animations, numbers were taken from def files
 	{
 		MOVING=0,

client/battle/CBattleAnimations.cpp

@@ -20,6 +20,16 @@
 #include "../../lib/BattleState.h"
 #include "../../lib/CTownHandler.h"
 
+/*
+ * CBattleAnimations.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
 CBattleAnimation::CBattleAnimation(CBattleInterface * _owner)
     : owner(_owner), ID(_owner->animIDhelper++)
 {}
@@ -302,8 +312,8 @@ void CMeleeAttackAnimation::endAnim()
 
 bool CMovementAnimation::shouldRotate()
 {
-	Point begPosition = CClickableHex::getXYUnitAnim(oldPos, stack->attackerOwned, stack, owner);
+	Point begPosition = CClickableHex::getXYUnitAnim(oldPos, stack, owner);
-	Point endPosition = CClickableHex::getXYUnitAnim(nextHex, stack->attackerOwned, stack, owner);
+	Point endPosition = CClickableHex::getXYUnitAnim(nextHex, stack, owner);
 
 	if((begPosition.x > endPosition.x) && owner->creDir[stack->ID] == true)
 	{
@@ -361,8 +371,8 @@ bool CMovementAnimation::init()
 		owner->moveSoundHander = CCS->soundh->playSound(battle_sound(stack->getCreature(), move), -1);
 	}
 
-	Point begPosition = CClickableHex::getXYUnitAnim(oldPos, stack->attackerOwned, stack, owner);
+	Point begPosition = CClickableHex::getXYUnitAnim(oldPos, stack, owner);
-	Point endPosition = CClickableHex::getXYUnitAnim(nextHex, stack->attackerOwned, stack, owner);
+	Point endPosition = CClickableHex::getXYUnitAnim(nextHex, stack, owner);
 
 	timeToMove = AnimationControls::getMovementDuration(stack->getCreature());
 
@@ -395,7 +405,7 @@ void CMovementAnimation::nextFrame()
 	if(progress >= 1.0)
 	{
 		// Sets the position of the creature animation sprites
-		Point coords = CClickableHex::getXYUnitAnim(nextHex, owner->creDir[stack->ID], stack, owner);
+		Point coords = CClickableHex::getXYUnitAnim(nextHex, stack, owner);
 		myAnim->pos = coords;
 
 		// true if creature haven't reached the final destination hex
@@ -406,13 +416,6 @@ void CMovementAnimation::nextFrame()
 			oldPos = nextHex;
 			nextHex = destTiles[curentMoveIndex];
 
-			// update position of double wide creatures
-			bool twoTiles = stack->doubleWide();
-			if(twoTiles && bool(stack->attackerOwned) && (owner->creDir[stack->ID] != bool(stack->attackerOwned) )) //big attacker creature is reversed
-				myAnim->pos.x -= 44;
-			else if(twoTiles && (! bool(stack->attackerOwned) ) && (owner->creDir[stack->ID] != bool(stack->attackerOwned) )) //big defender creature is reversed
-				myAnim->pos.x += 44;
-
 			// re-init animation
 			for(auto & elem : owner->pendingAnims)
 			{
@@ -432,7 +435,7 @@ void CMovementAnimation::endAnim()
 {
 	assert(stack);
 
-	myAnim->pos = CClickableHex::getXYUnitAnim(nextHex, owner->creDir[stack->ID], stack, owner);
+	myAnim->pos = CClickableHex::getXYUnitAnim(nextHex, stack, owner);
 	CBattleAnimation::endAnim();
 
 	owner->addNewAnim(new CMovementEndAnimation(owner, stack, nextHex));
@@ -540,8 +543,6 @@ bool CReverseAnimation::init()
 	if(!priority && !isEarliest(false))
 		return false;
 
-	//myAnim->pos = CClickableHex::getXYUnitAnim(hex, owner->creDir[stack->ID], stack, owner);
-
 	if(myAnim->framesInGroup(CCreatureAnim::TURN_L))
 	{
 		myAnim->setType(CCreatureAnim::TURN_L);
@@ -567,21 +568,7 @@ void CReverseAnimation::rotateStack(CBattleInterface * owner, const CStack * sta
 {
 	owner->creDir[stack->ID] = !owner->creDir[stack->ID];
 
-	owner->creAnims[stack->ID]->pos = CClickableHex::getXYUnitAnim(hex, owner->creDir[stack->ID], stack, owner);
+	owner->creAnims[stack->ID]->pos = CClickableHex::getXYUnitAnim(hex, stack, owner);
-
-	if(stack->doubleWide())
-	{
-		if(stack->attackerOwned)
-		{
-			if(!owner->creDir[stack->ID])
-				owner->creAnims[stack->ID]->pos.x -= 44;
-		}
-		else
-		{
-			if(owner->creDir[stack->ID])
-				owner->creAnims[stack->ID]->pos.x += 44;
-		}
-	}
 }
 
 void CReverseAnimation::setupSecondPart()
@@ -659,7 +646,7 @@ bool CShootingAnimation::init()
 	fromPos = owner->creAnims[spi.stackID]->pos.topLeft();
 	//xycoord = CClickableHex::getXYUnitAnim(shooter->position, true, shooter, owner);
 
-	destPos = CClickableHex::getXYUnitAnim(dest, false, attackedStack, owner);
+	destPos = CClickableHex::getXYUnitAnim(dest, attackedStack, owner);
 
 	// to properly translate coordinates when shooter is rotated
 	int multiplier = spi.reverse ? -1 : 1;

client/battle/CBattleInterface.cpp

@@ -802,7 +802,7 @@ void CBattleInterface::show(SDL_Surface * to)
 	{
 		for(size_t v=0; v<elem.size(); ++v)
 		{
-			creAnims[elem[v]->ID]->nextFrame(to, creAnims[elem[v]->ID]->pos.x, creAnims[elem[v]->ID]->pos.y, creDir[elem[v]->ID]);
+			creAnims[elem[v]->ID]->nextFrame(to, creDir[elem[v]->ID]);
 			creAnims[elem[v]->ID]->incrementFrame(float(GH.mainFPSmng->getElapsedMilliseconds()) / 1000);
 		}
 	}
@@ -986,8 +986,7 @@ void CBattleInterface::showAliveStacks(std::vector<const CStack *> *aliveStacks,
 
 void CBattleInterface::showObstacles(std::multimap<BattleHex, int> *hexToObstacle, std::vector<shared_ptr<const CObstacleInstance> > &obstacles, int hex, SDL_Surface *to)
 {
-	std::pair<std::multimap<BattleHex, int>::const_iterator, std::multimap<BattleHex, int>::const_iterator> obstRange =
+	auto obstRange = hexToObstacle->equal_range(hex);
-		hexToObstacle->equal_range(hex);
 
 	for(auto it = obstRange.first; it != obstRange.second; ++it)
 	{
@@ -1385,7 +1384,9 @@ void CBattleInterface::bConsoleDownf()
 
 void CBattleInterface::newStack(const CStack * stack)
 {
-	Point coords = CClickableHex::getXYUnitAnim(stack->position, stack->owner == attackingHeroInstance->tempOwner, stack, this);
+	creDir[stack->ID] = stack->attackerOwned; // must be set before getting stack position
+
+	Point coords = CClickableHex::getXYUnitAnim(stack->position, stack, this);
 
 	if(stack->position < 0) //turret
 	{
@@ -1425,7 +1426,6 @@ void CBattleInterface::newStack(const CStack * stack)
 	creAnims[stack->ID]->pos.y = coords.y;
 	creAnims[stack->ID]->pos.w = creAnims[stack->ID]->getWidth();
 	creAnims[stack->ID]->setType(CCreatureAnim::HOLDING);
-	creDir[stack->ID] = stack->attackerOwned;
 	
 }
 
@@ -1680,7 +1680,7 @@ void CBattleInterface::spellCast( const BattleSpellCast * sc )
 			//initial variables
 			std::string animToDisplay;
 			Point srccoord = (sc->side ? Point(770, 60) : Point(30, 60)) + pos;
-			Point destcoord = CClickableHex::getXYUnitAnim(sc->tile, !sc->side, curInt->cb->battleGetStackByPos(sc->tile), this); //position attacked by arrow
+			Point destcoord = CClickableHex::getXYUnitAnim(sc->tile, curInt->cb->battleGetStackByPos(sc->tile), this); //position attacked by arrow
 			destcoord.x += 250; destcoord.y += 240;
 
 			//animation angle
@@ -2241,7 +2241,7 @@ void CBattleInterface::showAliveStack(const CStack *stack, SDL_Surface * to)
 	if(creAnims.find(ID) == creAnims.end()) //eg. for summoned but not yet handled stacks
 		return;
 
-	creAnims[ID]->nextFrame(to, creAnims[ID]->pos.x, creAnims[ID]->pos.y, creDir[ID], nullptr);
+	creAnims[ID]->nextFrame(to, creDir[ID]);
 	creAnims[ID]->incrementFrame(float(GH.mainFPSmng->getElapsedMilliseconds()) / 1000);
 
 	//printing amount

client/battle/CBattleInterface.h

@@ -50,12 +50,6 @@ struct InfoAboutHero;
 struct BattleAction;
 class CBattleGameInterface;
 
-/// Class which manages the locked hex fields that are blocked e.g. by obstacles
-class CBattleObstacle
-{
-	std::vector<int> lockedHexes;
-};
-
 /// Small struct which contains information about the id of the attacked stack, the damage dealt,...
 struct StackAttackedInfo
 {
@@ -217,14 +211,13 @@ public:
 	int getAnimSpeed() const; //speed of animation; range 1..100
 
 	std::vector<CClickableHex*> bfield; //11 lines, 17 hexes on each
-	//std::vector< CBattleObstacle * > obstacles; //vector of obstacles on the battlefield
 	SDL_Surface * cellBorder, * cellShade;
 	CondSh<BattleAction *> *givenCommand; //data != nullptr if we have i.e. moved current unit
 	bool myTurn; //if true, interface is active (commands can be ordered)
 	CBattleResultWindow * resWindow; //window of end of battle
 
 	bool moveStarted; //if true, the creature that is already moving is going to make its first step
-	int moveSoundHander;		  // sound handler used when moving a unit
+	int moveSoundHander; // sound handler used when moving a unit
 
 	const BattleResult * bresult; //result of a battle; if non-zero then display when all animations end
 

client/battle/CBattleInterfaceClasses.cpp

@@ -25,6 +25,16 @@
 #include "../CCreatureWindow.h"
 #include "../CMessage.h"
 
+/*
+ * CBattleInterfaceClasses.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
 void CBattleConsole::showAll(SDL_Surface * to)
 {
 	Point textPos(pos.x + pos.w/2, pos.y + 17);
@@ -473,7 +483,7 @@ void CBattleResultWindow::bExitf()
 	CCS->videoh->close();
 }
 
-Point CClickableHex::getXYUnitAnim(const int & hexNum, const bool & attacker, const CStack * stack, const CBattleInterface * cbi)
+Point CClickableHex::getXYUnitAnim(BattleHex hexNum, const CStack * stack, CBattleInterface * cbi)
 {
 	Point ret(-500, -500); //returned value
 	if(stack && stack->position < 0) //creatures in turrets
@@ -493,26 +503,31 @@ Point CClickableHex::getXYUnitAnim(const int & hexNum, const bool & attacker, co
 	}
 	else
 	{
-		ret.y = -139 + 42 * (hexNum/GameConstants::BFIELD_WIDTH); //counting y
+		static const Point basePos(-190, -139); // position of creature in topleft corner
+		static const int imageShiftX = 30; // X offset to base pos for facing right stacks, negative for facing left
+
+		ret.y = basePos.y + 42 * hexNum.getY(); //counting y
 		//counting x
-		if(attacker)
+		if(cbi->creDir[stack->ID])
 		{
-			ret.x = -160 + 22 * ( ((hexNum/GameConstants::BFIELD_WIDTH) + 1)%2 ) + 44 * (hexNum % GameConstants::BFIELD_WIDTH);
+			ret.x = basePos.x + imageShiftX + 22 * ( (hexNum.getY() + 1)%2 ) + 44 * hexNum.getX();
 		}
 		else
 		{
-			ret.x = -219 + 22 * ( ((hexNum/GameConstants::BFIELD_WIDTH) + 1)%2 ) + 44 * (hexNum % GameConstants::BFIELD_WIDTH);
+			ret.x = basePos.x - imageShiftX + 22 * ( (hexNum.getY() + 1)%2 ) + 44 * hexNum.getX();
 		}
 		//shifting position for double - hex creatures
 		if(stack && stack->doubleWide())
 		{
-			if(attacker)
+			if(stack->attackerOwned)
 			{
-				ret.x -= 44;
+				if(cbi->creDir[stack->ID])
+					ret.x -= 44;
 			}
 			else
 			{
-				ret.x += 45;
+				if(!cbi->creDir[stack->ID])
+					ret.x += 44;
 			}
 		}
 	}

client/battle/CBattleInterfaceClasses.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "../gui/CIntObject.h"
+#include "../../lib/BattleHex.h"
 
 struct SDL_Surface;
 class CDefHandler;
@@ -110,7 +111,7 @@ public:
 	//CStack * ourStack;
 	bool hovered, strictHovered; //for determining if hex is hovered by mouse (this is different problem than hex's graphic hovering)
 	CBattleInterface * myInterface; //interface that owns me
-	static Point getXYUnitAnim(const int & hexNum, const bool & attacker, const CStack * creature, const CBattleInterface * cbi); //returns (x, y) of left top corner of animation
+	static Point getXYUnitAnim(BattleHex hexNum, const CStack * creature, CBattleInterface * cbi); //returns (x, y) of left top corner of animation
 
 	//for user interactions
 	void hover (bool on);

client/battle/CCreatureAnimation.cpp

@@ -289,7 +289,7 @@ std::array<SDL_Color, 8> CCreatureAnimation::genSpecialPalette()
 }
 
 template<int bpp>
-void CCreatureAnimation::nextFrameT(SDL_Surface * dest, int x, int y, bool rotate, SDL_Rect * destRect /*= nullptr*/)
+void CCreatureAnimation::nextFrameT(SDL_Surface * dest, bool rotate)
 {
 	assert(dataOffsets.count(type) && dataOffsets.at(type).size() > size_t(currentFrame));
 
@@ -322,13 +322,13 @@ void CCreatureAnimation::nextFrameT(SDL_Surface * dest, int x, int y, bool rotat
 		//NOTE: if this loop will be optimized to skip empty lines - recheck this read access
 		ui8 * lineData = pixelData.get() + baseOffset + reader.readUInt32();
 
-		size_t destX = x;
+		size_t destX = pos.x;
 		if (rotate)
 			destX += rightMargin + spriteWidth - 1;
 		else
 			destX += leftMargin;
 
-		size_t destY = y + topMargin + i;
+		size_t destY = pos.y + topMargin + i;
 		size_t currentOffset = 0;
 		size_t totalRowLength = 0;
 
@@ -340,7 +340,7 @@ void CCreatureAnimation::nextFrameT(SDL_Surface * dest, int x, int y, bool rotat
 			if (type==0xFF)//Raw data
 			{
 				for (size_t j=0; j<length; j++)
-					putPixelAt<bpp>(dest, destX + (rotate?(-j):(j)), destY, lineData[currentOffset + j], specialPalette, destRect);
+					putPixelAt<bpp>(dest, destX + (rotate?(-j):(j)), destY, lineData[currentOffset + j], specialPalette);
 
 				currentOffset += length;
 			}
@@ -349,7 +349,7 @@ void CCreatureAnimation::nextFrameT(SDL_Surface * dest, int x, int y, bool rotat
 				if (type != 0) // transparency row, handle it here for speed
 				{
 					for (size_t j=0; j<length; j++)
-						putPixelAt<bpp>(dest, destX + (rotate?(-j):(j)), destY, type, specialPalette, destRect);
+						putPixelAt<bpp>(dest, destX + (rotate?(-j):(j)), destY, type, specialPalette);
 				}
 			}
 
@@ -359,13 +359,15 @@ void CCreatureAnimation::nextFrameT(SDL_Surface * dest, int x, int y, bool rotat
 	}
 }
 
-void CCreatureAnimation::nextFrame(SDL_Surface *dest, int x, int y, bool attacker, SDL_Rect * destRect)
+void CCreatureAnimation::nextFrame(SDL_Surface *dest, bool attacker)
 {
+	// Note: please notice that attacker value is inversed when passed further.
+	// This is intended behavior because "attacker" actually does not needs rotation
 	switch(dest->format->BytesPerPixel)
 	{
-	case 2: return nextFrameT<2>(dest, x, y, !attacker, destRect);
+	case 2: return nextFrameT<2>(dest, !attacker);
-	case 3: return nextFrameT<3>(dest, x, y, !attacker, destRect);
+	case 3: return nextFrameT<3>(dest, !attacker);
-	case 4: return nextFrameT<4>(dest, x, y, !attacker, destRect);
+	case 4: return nextFrameT<4>(dest, !attacker);
 	default:
         logGlobal->errorStream() << (int)dest->format->BitsPerPixel << " bpp is not supported!!!";
 	}
@@ -385,16 +387,9 @@ ui8 * CCreatureAnimation::getPixelAddr(SDL_Surface * dest, int X, int Y) const
 }
 
 template<int bpp>
-inline void CCreatureAnimation::putPixelAt(SDL_Surface * dest, int X, int Y, size_t index, const std::array<SDL_Color, 8> & special, SDL_Rect * destRect) const
+inline void CCreatureAnimation::putPixelAt(SDL_Surface * dest, int X, int Y, size_t index, const std::array<SDL_Color, 8> & special) const
 {
-	if (destRect == nullptr)
+	putPixel<bpp>(getPixelAddr(dest, X, Y), palette[index], index, special);
-		putPixel<bpp>(getPixelAddr(dest, X, Y), palette[index], index, special);
-	else
-	{
-		if ( X > destRect->x && X < destRect->w + destRect->x &&
-		     Y > destRect->y && Y < destRect->h + destRect->y )
-			putPixel<bpp>(getPixelAddr(dest, X, Y), palette[index], index, special);
-	}
 }
 
 template<int bpp>

client/battle/CCreatureAnimation.h

@@ -86,13 +86,13 @@ private:
 	ui8 * getPixelAddr(SDL_Surface * dest, int ftcpX, int ftcpY) const;
 
 	template<int bpp>
-	void putPixelAt(SDL_Surface * dest, int X, int Y, size_t index, const std::array<SDL_Color, 8> & special, SDL_Rect * destRect = nullptr) const;
+	void putPixelAt(SDL_Surface * dest, int X, int Y, size_t index, const std::array<SDL_Color, 8> & special) const;
 
 	template<int bpp>
 	void putPixel( ui8 * dest, const SDL_Color & color, size_t index, const std::array<SDL_Color, 8> & special) const;
 
 	template<int bpp>
-	void nextFrameT(SDL_Surface * dest, int x, int y, bool rotate, SDL_Rect * destRect = nullptr);
+	void nextFrameT(SDL_Surface * dest, bool rotate);
 
 	void endAnimation();
 
@@ -116,7 +116,7 @@ public:
 	void setType(CCreatureAnim::EAnimType type); //sets type of animation and cleares framecount
 	CCreatureAnim::EAnimType getType() const; //returns type of animation
 
-	void nextFrame(SDL_Surface * dest, int x, int y, bool rotate, SDL_Rect * destRect = nullptr);
+	void nextFrame(SDL_Surface * dest, bool rotate);
 
 	// should be called every frame, return true when animation was reset to beginning
 	bool incrementFrame(float timePassed);

config/schemas/faction.json

@@ -132,10 +132,13 @@
 						},
 						"dwellings" : {
 							"type" : "array",
+							"minItems" : 7,
+							"maxItems" : 7,
 							"description" : "Dwellings on adventure map",
 							"items" : {
 								"type" : "object",
 								"additionalProperties" : false,
+								"required" : [ "name", "graphics" ],
 								"properties" : {
 									"name":     { "type":"string" },
 									"graphics": { "type":"string" }

lib/GameConstants.h

@@ -642,7 +642,7 @@ namespace BattlefieldBI
 		LUCID_POOLS,
 		FIERY_FIELDS,
 		ROCKLANDS,
-		MAGIC_CLOUDS,
+		MAGIC_CLOUDS
 	};
 }
 
@@ -662,8 +662,21 @@ namespace Battle
 {
 	enum ActionType
 	{
-		END_TACTIC_PHASE = -2, INVALID = -1, NO_ACTION = 0, HERO_SPELL, WALK, DEFEND, RETREAT, SURRENDER, WALK_AND_ATTACK, SHOOT, WAIT, CATAPULT, MONSTER_SPELL, BAD_MORALE,
+		END_TACTIC_PHASE = -2,
-		STACK_HEAL, DAEMON_SUMMONING
+		INVALID = -1,
+		NO_ACTION = 0,
+		HERO_SPELL,
+		WALK, DEFEND,
+		RETREAT,
+		SURRENDER,
+		WALK_AND_ATTACK,
+		SHOOT,
+		WAIT,
+		CATAPULT,
+		MONSTER_SPELL,
+		BAD_MORALE,
+		STACK_HEAL,
+		DAEMON_SUMMONING
 	};
 }
 

test/CMakeLists.txt

@@ -11,7 +11,7 @@ set(test_SRCS
 )
 
 add_executable(vcmitest ${test_SRCS})
-target_link_libraries(vcmitest vcmi ${Boost_LIBRARIES})
+target_link_libraries(vcmitest vcmi ${Boost_LIBRARIES} ${RT_LIB} ${DL_LIB})
 add_test(vcmitest vcmitest)
 
 # Files to copy to the build directory

test/CVcmiTestConfig.h

@@ -1,3 +1,4 @@
+#pragma once
 
 /*
  * CVcmiTestConfig.h, part of VCMI engine
@@ -9,8 +10,6 @@
  *
  */
 
-#pragma once
-
 /// Global setup/tear down class for unit tests.
 class CVcmiTestConfig
 {

test/StdInc.cpp

@@ -1,2 +1,10 @@
 // Creates the precompiled header
 #include "StdInc.h"
+
+#ifndef _MSC_VER
+	// Should be defined only once, before #include of unit test header
+	#define BOOST_TEST_MODULE VcmiTest
+	#include <boost/test/unit_test.hpp>
+	#include "CVcmiTestConfig.h"
+	BOOST_GLOBAL_FIXTURE(CVcmiTestConfig);
+#endif

test/StdInc.h

@@ -6,9 +6,11 @@
 	#define BOOST_TEST_DYN_LINK
 #endif
 
-#define BOOST_TEST_MODULE VcmiTest
+#ifdef _MSC_VER
-#include <boost/test/unit_test.hpp>
+	#define BOOST_TEST_MODULE VcmiTest
-#include "CVcmiTestConfig.h"
+	#include <boost/test/unit_test.hpp>
-BOOST_GLOBAL_FIXTURE(CVcmiTestConfig);
+	#include "CVcmiTestConfig.h"
+	BOOST_GLOBAL_FIXTURE(CVcmiTestConfig);
+#endif