Changes for BattleInterface classes according to code review:

- added documentation comments for classes, members and methods
- added const specifier to methods where applicable
- renamed some methods with more clear name
- removed some commented-out or unused code

client/battle/BattleActionsController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleActionsController.cpp, part of VCMI engine
+ * BattleActionsController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -141,7 +141,7 @@ void BattleActionsController::enterCreatureCastingMode()
 	}
 }
 
-std::vector<PossiblePlayerBattleAction> BattleActionsController::getPossibleActionsForStack(const CStack *stack)
+std::vector<PossiblePlayerBattleAction> BattleActionsController::getPossibleActionsForStack(const CStack *stack) const
 {
 	BattleClientInterfaceData data; //hard to get rid of these things so for now they're required data to pass
 	data.creatureSpellToCast = owner->stacksController->activeStackSpellToCast();
@@ -742,7 +742,7 @@ bool BattleActionsController::isCastingPossibleHere(const CStack *sactive, const
 	return isCastingPossible;
 }
 
-bool BattleActionsController::canStackMoveHere(const CStack * stackToMove, BattleHex myNumber)
+bool BattleActionsController::canStackMoveHere(const CStack * stackToMove, BattleHex myNumber) const
 {
 	std::vector<BattleHex> acc = owner->curInt->cb->battleGetAvailableHexes(stackToMove);
 	BattleHex shiftedDest = myNumber.cloneInDirection(stackToMove->destShiftDir(), false);
@@ -762,12 +762,12 @@ void BattleActionsController::activateStack()
 		possibleActions = getPossibleActionsForStack(s);
 }
 
-bool BattleActionsController::spellcastingModeActive()
+bool BattleActionsController::spellcastingModeActive() const
 {
 	return spellDestSelectMode;
 }
 
-SpellID BattleActionsController::selectedSpell()
+SpellID BattleActionsController::selectedSpell() const
 {
 	if (!spellToCast)
 		return SpellID::NONE;

client/battle/BattleActionsController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleActionsController.h, part of VCMI engine
+ * BattleActionsController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -25,39 +25,72 @@ enum class MouseHoveredHexContext
 	OCCUPIED_HEX
 };
 
+/// Class that controls actions that can be performed by player, e.g. moving stacks, attacking, etc
+/// As well as all relevant feedback for these actions in user interface
 class BattleActionsController
 {
 	BattleInterface * owner;
 
-	std::vector<PossiblePlayerBattleAction> possibleActions; //all actions possible to call at the moment by player
+	/// all actions possible to call at the moment by player
-	std::vector<PossiblePlayerBattleAction> localActions; //actions possible to take on hovered hex
+	std::vector<PossiblePlayerBattleAction> possibleActions;
-	std::vector<PossiblePlayerBattleAction> illegalActions; //these actions display message in case of illegal target
-	PossiblePlayerBattleAction currentAction; //action that will be performed on l-click
-	PossiblePlayerBattleAction selectedAction; //last action chosen (and saved) by player
-	PossiblePlayerBattleAction illegalAction; //most likely action that can't be performed here
 
-	bool creatureCasting; //if true, stack currently aims to cats a spell
+	/// actions possible to take on hovered hex
-	bool spellDestSelectMode; //if true, player is choosing destination for his spell - only for GUI / console
+	std::vector<PossiblePlayerBattleAction> localActions;
-	std::shared_ptr<BattleAction> spellToCast; //spell for which player is choosing destination
+
-	const CSpell *currentSpell; //spell pointer for convenience
+	/// these actions display message in case of illegal target
+	std::vector<PossiblePlayerBattleAction> illegalActions;
+
+	/// action that will be performed on l-click
+	PossiblePlayerBattleAction currentAction;
+
+	/// last action chosen (and saved) by player
+	PossiblePlayerBattleAction selectedAction;
+
+	/// if there are not possible actions to choose from, this action should be show as "illegal" in UI
+	PossiblePlayerBattleAction illegalAction;
+
+	/// if true, stack currently aims to cats a spell
+	bool creatureCasting;
+
+	/// if true, player is choosing destination for his spell - only for GUI / console
+	bool spellDestSelectMode;
+
+	/// spell for which player is choosing destination
+	std::shared_ptr<BattleAction> spellToCast;
+
+	/// spell for which player is choosing destination, pointer for convenience
+	const CSpell *currentSpell;
+
+	/// cached message that was set by this class in status bar
+	std::string currentConsoleMsg;
 
 	bool isCastingPossibleHere (const CStack *sactive, const CStack *shere, BattleHex myNumber);
-	bool canStackMoveHere (const CStack *sactive, BattleHex MyNumber); //TODO: move to BattleState / callback
+	bool canStackMoveHere (const CStack *sactive, BattleHex MyNumber) const; //TODO: move to BattleState / callback
-
+	std::vector<PossiblePlayerBattleAction> getPossibleActionsForStack (const CStack *stack) const; //called when stack gets its turn
-	std::vector<PossiblePlayerBattleAction> getPossibleActionsForStack (const CStack *stack); //called when stack gets its turn
 	void reorderPossibleActionsPriority(const CStack * stack, MouseHoveredHexContext context);
 
-	std::string currentConsoleMsg;
 public:
 	BattleActionsController(BattleInterface * owner);
 
+	/// initialize list of potential actions for new active stack
 	void activateStack();
-	void endCastingSpell(); //ends casting spell (eg. when spell has been cast or canceled)
+
+	/// initialize potential actions for spells that can be cast by active stack
 	void enterCreatureCastingMode();
 
-	SpellID selectedSpell();
+	/// initialize potential actions for selected spell
-	bool spellcastingModeActive();
+	void castThisSpell(SpellID spellID);
-	void castThisSpell(SpellID spellID); //called when player has chosen a spell from spellbook
+
+	/// ends casting spell (eg. when spell has been cast or canceled)
+	void endCastingSpell();
+
+	/// update UI (e.g. status bar/cursor) according to new active hex
 	void handleHex(BattleHex myNumber, int eventType);
 
+	/// returns currently selected spell or SpellID::NONE on error
+	SpellID selectedSpell() const;
+
+	/// returns true if UI is currently in target selection mode
+	bool spellcastingModeActive() const;
+
 };

client/battle/BattleAnimationClasses.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleAnimations.cpp, part of VCMI engine
+ * BattleAnimationClasses.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -855,7 +855,7 @@ CShootingAnimation::CShootingAnimation(BattleInterface * _owner, const CStack *
 
 void CShootingAnimation::createProjectile(const Point & from, const Point & dest) const
 {
-	owner->projectilesController->createProjectile(attackingStack, attackedStack, from, dest);
+	owner->projectilesController->createProjectile(attackingStack, from, dest);
 }
 
 uint32_t CShootingAnimation::getAttackClimaxFrame() const
@@ -970,7 +970,7 @@ CCreatureAnim::EAnimType CCastAnimation::getDownwardsGroup() const
 void CCastAnimation::createProjectile(const Point & from, const Point & dest) const
 {
 	if (!spell->animationInfo.projectile.empty())
-		owner->projectilesController->createSpellProjectile(attackingStack, attackedStack, from, dest, spell);
+		owner->projectilesController->createSpellProjectile(attackingStack, from, dest, spell);
 }
 
 uint32_t CCastAnimation::getAttackClimaxFrame() const
@@ -1071,7 +1071,7 @@ bool CPointEffectAnimation::init()
 		else
 		{
 			const CStack * destStack = owner->getCurrentPlayerInterface()->cb->battleGetStackByPos(battlehexes[i], false);
-			Rect tilePos = owner->fieldController->hexPosition(battlehexes[i]);
+			Rect tilePos = owner->fieldController->hexPositionAbsolute(battlehexes[i]);
 
 			be.x = tilePos.x + tilePos.w/2 - first->width()/2;
 

client/battle/BattleAnimationClasses.h

@@ -1,5 +1,5 @@
 /*
- * CBattleAnimations.h, part of VCMI engine
+ * BattleAnimations.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleControlPanel.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleControlPanel.cpp, part of VCMI engine
+ * BattleControlPanel.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleControlPanel.h

@@ -1,5 +1,5 @@
 /*
- * CBattleControlPanel.h, part of VCMI engine
+ * BattleControlPanel.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -20,6 +20,7 @@ class CButton;
 class BattleInterface;
 class BattleConsole;
 
+/// GUI object that handles functionality of panel at the bottom of combat screen
 class BattleControlPanel : public CIntObject
 {
 	BattleInterface * owner;
@@ -38,7 +39,7 @@ class BattleControlPanel : public CIntObject
 	std::shared_ptr<CButton> btactNext;
 	std::shared_ptr<CButton> btactEnd;
 
-	//button handle funcs:
+	/// button press handling functions
 	void bOptionsf();
 	void bSurrenderf();
 	void bFleef();
@@ -51,20 +52,23 @@ class BattleControlPanel : public CIntObject
 	void bTacticNextStack();
 	void bTacticPhaseEnd();
 
-	void reallyFlee(); //performs fleeing without asking player
+	/// functions for handling actions after they were confirmed by popup window
-	void reallySurrender(); //performs surrendering without asking player
+	void reallyFlee();
+	void reallySurrender();
 
 public:
 	std::shared_ptr<BattleConsole> console;
 
-	// block all UI elements, e.g. during enemy turn
+	/// block all UI elements when player is not allowed to act, e.g. during enemy turn
-	// unlike activate/deactivate this method will correctly grey-out all elements
 	void blockUI(bool on);
 
 	void show(SDL_Surface * to) override;
 	void showAll(SDL_Surface * to) override;
 
+	/// Toggle UI to displaying tactics phase
 	void tacticPhaseStarted();
+
+	/// Toggle UI to displaying battle log in place of tactics UI
 	void tacticPhaseEnded();
 
 	BattleControlPanel(BattleInterface * owner, const Point & position);

client/battle/BattleEffectsController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleEffectsController.cpp, part of VCMI engine
+ * BattleEffectsController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleEffectsController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleEffectsController.h, part of VCMI engine
+ * BattleEffectsController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -57,6 +57,7 @@ struct BattleEffect
 	BattleHex position; //Indicates if effect which hex the effect is drawn on
 };
 
+/// Controls rendering of effects in battle, e.g. from spells, abilities and various other actions like morale
 class BattleEffectsController
 {
 	BattleInterface * owner;

client/battle/BattleFieldController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleFieldController.cpp, part of VCMI engine
+ * BattleFieldController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -79,7 +79,7 @@ BattleFieldController::BattleFieldController(BattleInterface * owner):
 	{
 		auto hex = std::make_shared<ClickableHex>();
 		hex->myNumber = h;
-		hex->pos = hexPosition(h);
+		hex->pos = hexPositionAbsolute(h);
 		hex->myInterface = owner;
 		bfield.push_back(hex);
 	}
@@ -162,7 +162,7 @@ void BattleFieldController::redrawBackgroundWithHexes()
 
 void BattleFieldController::showHighlightedHex(Canvas & canvas, BattleHex hex, bool darkBorder)
 {
-	Point hexPos = hexPosition(hex).topLeft();
+	Point hexPos = hexPositionAbsolute(hex).topLeft();
 
 	canvas.draw(cellShade, hexPos);
 	if(!darkBorder && settings["battle"]["cellBorders"].Bool())
@@ -278,7 +278,7 @@ Rect BattleFieldController::hexPositionLocal(BattleHex hex) const
 	return Rect(x, y, w, h);
 }
 
-Rect BattleFieldController::hexPosition(BattleHex hex) const
+Rect BattleFieldController::hexPositionAbsolute(BattleHex hex) const
 {
 	return hexPositionLocal(hex) + owner->pos.topLeft();
 }
@@ -299,7 +299,7 @@ BattleHex BattleFieldController::getHoveredHex()
 
 void BattleFieldController::setBattleCursor(BattleHex myNumber)
 {
-	Rect hoveredHexPos = hexPosition(myNumber);
+	Rect hoveredHexPos = hexPositionAbsolute(myNumber);
 	CCursorHandler *cursor = CCS->curh;
 
 	const double subdividingAngle = 2.0*M_PI/6.0; // Divide a hex into six sectors.

client/battle/BattleFieldController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleFieldController.h, part of VCMI engine
+ * BattleFieldController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -27,6 +27,7 @@ class Canvas;
 class IImage;
 class BattleInterface;
 
+/// Handles battlefield grid as well as rendering of background layer of battle interface
 class BattleFieldController : public CIntObject
 {
 	BattleInterface * owner;
@@ -35,19 +36,22 @@ class BattleFieldController : public CIntObject
 	std::shared_ptr<IImage> cellBorder;
 	std::shared_ptr<IImage> cellShade;
 
-	std::unique_ptr<Canvas> cellBorders;
-
 	/// Canvas that contains background, hex grid (if enabled), absolute obstacles and movement range of active stack
 	std::unique_ptr<Canvas> backgroundWithHexes;
 
-	//BattleHex previouslyHoveredHex; //number of hex that was hovered by the cursor a while ago
+	/// Canvas that contains cell borders of all tiles in the battlefield
-	//BattleHex currentlyHoveredHex; //number of hex that is supposed to be hovered (for a while it may be inappropriately set, but will be renewed soon)
+	std::unique_ptr<Canvas> cellBorders;
-	BattleHex attackingHex; //hex from which the stack would perform attack with current cursor
 
-	std::vector<BattleHex> occupyableHexes; //hexes available for active stack
+	/// hex from which the stack would perform attack with current cursor
-	std::array<bool, GameConstants::BFIELD_SIZE> stackCountOutsideHexes; // hexes that when in front of a unit cause it's amount box to move back
+	BattleHex attackingHex;
 
-	std::vector<std::shared_ptr<ClickableHex>> bfield; //11 lines, 17 hexes on each
+	/// hexes to which currently active stack can move
+	std::vector<BattleHex> occupyableHexes;
+
+	/// hexes that when in front of a unit cause it's amount box to move back
+	std::array<bool, GameConstants::BFIELD_SIZE> stackCountOutsideHexes;
+
+	std::vector<std::shared_ptr<ClickableHex>> bfield;
 
 	void showHighlightedHex(Canvas & to, BattleHex hex, bool darkBorder);
 
@@ -65,14 +69,24 @@ public:
 	void redrawBackgroundWithHexes();
 	void renderBattlefield(Canvas & canvas);
 
+	/// Returns position of hex relative to owner (BattleInterface)
 	Rect hexPositionLocal(BattleHex hex) const;
-	Rect hexPosition(BattleHex hex) const;
+
+	/// Returns position of hex relative to game window
+	Rect hexPositionAbsolute(BattleHex hex) const;
+
+	/// Checks whether selected pixel is transparent, uses local coordinates of a hex
 	bool isPixelInHex(Point const & position);
 
+	/// Returns ID of currently hovered hex or BattleHex::INVALID if none
 	BattleHex getHoveredHex();
 
+	/// returns true if selected tile can be attacked in melee by current stack
+	bool isTileAttackable(const BattleHex & number) const;
+
+	/// returns true if stack should render its stack count image in default position - outside own hex
+	bool stackCountOutsideHex(const BattleHex & number) const;
+
 	void setBattleCursor(BattleHex myNumber);
 	BattleHex fromWhichHexAttack(BattleHex myNumber);
-	bool isTileAttackable(const BattleHex & number) const;
-	bool stackCountOutsideHex(const BattleHex & number) const;
 };

client/battle/BattleInterface.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleInterface.cpp, part of VCMI engine
+ * BattleInterface.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -514,7 +514,7 @@ void BattleInterface::spellCast(const BattleSpellCast * sc)
 			Point destcoord = stacksController->getStackPositionAtHex(sc->tile, target); //position attacked by projectile
 			destcoord += Point(250, 240); // FIXME: what are these constants?
 
-			projectilesController->createSpellProjectile( nullptr, target, srccoord, destcoord, spell);
+			projectilesController->createSpellProjectile( nullptr, srccoord, destcoord, spell);
 			projectilesController->emitStackProjectile( nullptr );
 
 			// wait fo projectile to end

client/battle/BattleInterface.h

@@ -1,5 +1,5 @@
 /*
- * CBattleInterface.h, part of VCMI engine
+ * BattleInterface.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -182,6 +182,7 @@ public:
 	const CGHeroInstance *currentHero() const;
 	InfoAboutHero enemyHero() const;
 
+	// TODO: cleanup this list
 	friend class CPlayerInterface;
 	friend class CInGameConsole;
 	friend class StackQueue;

client/battle/BattleInterfaceClasses.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleInterfaceClasses.cpp, part of VCMI engine
+ * BattleInterfaceClasses.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleInterfaceClasses.h

@@ -1,5 +1,5 @@
 /*
- * CBattleInterfaceClasses.h, part of VCMI engine
+ * BattleInterfaceClasses.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleObstacleController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleObstacleController.cpp, part of VCMI engine
+ * BattleObstacleController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -182,7 +182,7 @@ Point BattleObstacleController::getObstaclePosition(std::shared_ptr<IImage> imag
 {
 	int offset = obstacle.getAnimationYOffset(image->height());
 
-	Rect r = owner->fieldController->hexPosition(obstacle.pos);
+	Rect r = owner->fieldController->hexPositionAbsolute(obstacle.pos);
 	r.y += 42 - image->height() + offset;
 
 	return r.topLeft();

client/battle/BattleObstacleController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleObstacleController.h, part of VCMI engine
+ * BattleObstacleController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -24,16 +24,20 @@ class BattleInterface;
 class BattleRenderer;
 struct Point;
 
+/// Controls all currently active projectiles on the battlefield
+/// (with exception of moat, which is apparently handled by siege controller)
 class BattleObstacleController
 {
-	std::map<std::string, std::shared_ptr<CAnimation>> animationsCache;
-
 	BattleInterface * owner;
 
+	/// cached animations of all obstacles in current battle
+	std::map<std::string, std::shared_ptr<CAnimation>> animationsCache;
+
+	/// list of all obstacles that are currently being rendered
 	std::map<si32, std::shared_ptr<CAnimation>> obstacleAnimations;
 
-	// semi-debug member, contains obstacles that should not yet be visible due to ongoing placement animation
+	/// semi-debug member, contains obstacles that should not yet be visible due to ongoing placement animation
-	// used only for sanity checks to ensure that there are no invisible obstacles
+	/// used only for sanity checks to ensure that there are no invisible obstacles
 	std::vector<si32> obstaclesBeingPlaced;
 
 	void loadObstacleImage(const CObstacleInstance & oi);
@@ -44,9 +48,12 @@ class BattleObstacleController
 public:
 	BattleObstacleController(BattleInterface * owner);
 
+	/// call-in from network pack, add newly placed obstacles with any required animations
 	void obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> & oi);
-	void showObstacles(SDL_Surface *to, std::vector<std::shared_ptr<const CObstacleInstance>> &obstacles);
+
+	/// renders all "absolute" obstacles
 	void showAbsoluteObstacles(Canvas & canvas, const Point & offset);
 
+	/// adds all non-"absolute" visible obstacles for rendering
 	void collectRenderableObjects(BattleRenderer & renderer);
 };

client/battle/BattleProjectileController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleProjectileController.cpp, part of VCMI engine
+ * BattleProjectileController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -239,7 +239,7 @@ void BattleProjectileController::showProjectiles(Canvas & canvas)
 	}
 }
 
-bool BattleProjectileController::hasActiveProjectile(const CStack * stack)
+bool BattleProjectileController::hasActiveProjectile(const CStack * stack) const
 {
 	int stackID = stack ? stack->ID : -1;
 
@@ -312,10 +312,8 @@ void BattleProjectileController::createCatapultProjectile(const CStack * shooter
 	projectiles.push_back(std::shared_ptr<ProjectileBase>(catapultProjectile));
 }
 
-void BattleProjectileController::createProjectile(const CStack * shooter, const CStack * target, Point from, Point dest)
+void BattleProjectileController::createProjectile(const CStack * shooter, Point from, Point dest)
 {
-	assert(target);
-
 	const CCreature *shooterInfo = getShooter(shooter);
 
 	std::shared_ptr<ProjectileBase> projectile;
@@ -351,7 +349,7 @@ void BattleProjectileController::createProjectile(const CStack * shooter, const
 	projectiles.push_back(projectile);
 }
 
-void BattleProjectileController::createSpellProjectile(const CStack * shooter, const CStack * target, Point from, Point dest, const CSpell * spell)
+void BattleProjectileController::createSpellProjectile(const CStack * shooter, Point from, Point dest, const CSpell * spell)
 {
 	double projectileAngle = std::abs(atan2(dest.x - from.x, dest.y - from.y));
 	std::string animToDisplay = spell->animationInfo.selectProjectile(projectileAngle);

client/battle/BattleProjectileController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleSiegeController.h, part of VCMI engine
+ * BattleSiegeController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -25,7 +25,7 @@ class CAnimation;
 class Canvas;
 class BattleInterface;
 
-/// Small struct which contains information about the position and the velocity of a projectile
+/// Base class for projectiles
 struct ProjectileBase
 {
 	virtual ~ProjectileBase() = default;
@@ -40,6 +40,7 @@ struct ProjectileBase
 	bool playing;  // if set to true, projectile animation is playing, e.g. flying to target
 };
 
+/// Projectile for most shooters - render pre-selected frame moving in straight line from origin to destination
 struct ProjectileMissile : ProjectileBase
 {
 	void show(Canvas & canvas) override;
@@ -49,12 +50,14 @@ struct ProjectileMissile : ProjectileBase
 	bool reverse;  // if true, projectile will be flipped by vertical axis
 };
 
+/// Projectile for spell - render animation moving in straight line from origin to destination
 struct ProjectileAnimatedMissile : ProjectileMissile
 {
 	void show(Canvas & canvas) override;
 	float frameProgress;
 };
 
+/// Projectile for catapult - render spinning projectile moving at parabolic trajectory to its destination
 struct ProjectileCatapult : ProjectileBase
 {
 	void show(Canvas & canvas) override;
@@ -63,6 +66,7 @@ struct ProjectileCatapult : ProjectileBase
 	int frameNum;  // frame to display from projectile animation
 };
 
+/// Projectile for mages/evil eye - render ray expanding from origin position to destination
 struct ProjectileRay : ProjectileBase
 {
 	void show(Canvas & canvas) override;
@@ -70,6 +74,8 @@ struct ProjectileRay : ProjectileBase
 	std::vector<CCreature::CreatureAnimation::RayColor> rayConfig;
 };
 
+/// Class that manages all ongoing projectiles in the game
+/// ... even though in H3 only 1 projectile can be on screen at any point of time
 class BattleProjectileController
 {
 	BattleInterface * owner;
@@ -77,9 +83,6 @@ class BattleProjectileController
 	/// all projectiles loaded during current battle
 	std::map<std::string, std::shared_ptr<CAnimation>> projectilesCache;
 
-//	std::map<int, std::shared_ptr<CAnimation>> idToProjectile;
-//	std::map<int, std::vector<CCreature::CreatureAnimation::RayColor>> idToRay;
-
 	/// projectiles currently flying on battlefield
 	std::vector<std::shared_ptr<ProjectileBase>> projectiles;
 
@@ -96,14 +99,21 @@ class BattleProjectileController
 
 	int computeProjectileFrameID( Point from, Point dest, const CStack * stack);
 	int computeProjectileFlightTime( Point from, Point dest, double speed);
+
 public:
 	BattleProjectileController(BattleInterface * owner);
 
+	/// renders all currently active projectiles
 	void showProjectiles(Canvas & canvas);
 
-	bool hasActiveProjectile(const CStack * stack);
+	/// returns true if stack has projectile that is yet to hit target
+	bool hasActiveProjectile(const CStack * stack) const;
+
+	/// starts rendering previously created projectile
 	void emitStackProjectile(const CStack * stack);
-	void createProjectile(const CStack * shooter, const CStack * target, Point from, Point dest);
+
-	void createSpellProjectile(const CStack * shooter, const CStack * target, Point from, Point dest, const CSpell * spell);
+	/// creates (but not emits!) projectile and initializes it based on parameters
+	void createProjectile(const CStack * shooter, Point from, Point dest);
+	void createSpellProjectile(const CStack * shooter, Point from, Point dest, const CSpell * spell);
 	void createCatapultProjectile(const CStack * shooter, Point from, Point dest);
 };

client/battle/BattleRenderer.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleFieldController.cpp, part of VCMI engine
+ * BattleFieldController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleRenderer.h

@@ -1,5 +1,5 @@
 /*
- * CBattleFieldController.h, part of VCMI engine
+ * BattleFieldController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -11,19 +11,7 @@
 
 #include "../../lib/battle/BattleHex.h"
 
-VCMI_LIB_NAMESPACE_BEGIN
-
-//class CStack;
-
-VCMI_LIB_NAMESPACE_END
-
-//struct SDL_Surface;
-//struct Rect;
-//struct Point;
-//
-//class CClickableHex;
 class Canvas;
-//class IImage;
 class BattleInterface;
 
 enum class EBattleFieldLayer {

client/battle/BattleSiegeController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleSiegeController.cpp, part of VCMI engine
+ * BattleSiegeController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleSiegeController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleObstacleController.h, part of VCMI engine
+ * BattleObstacleController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *

client/battle/BattleStacksController.cpp

@@ -1,5 +1,5 @@
 /*
- * CBattleStacksController.cpp, part of VCMI engine
+ * BattleStacksController.cpp, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -435,7 +435,7 @@ void BattleStacksController::stackAttacking( const CStack *attacker, BattleHex d
 	//waitForAnims();
 }
 
-bool BattleStacksController::shouldRotate(const CStack * stack, const BattleHex & oldPos, const BattleHex & nextHex)
+bool BattleStacksController::shouldRotate(const CStack * stack, const BattleHex & oldPos, const BattleHex & nextHex) const
 {
 	Point begPosition = getStackPositionAtHex(oldPos,stack);
 	Point endPosition = getStackPositionAtHex(nextHex, stack);
@@ -528,19 +528,19 @@ void BattleStacksController::setSelectedStack(const CStack *stack)
 	selectedStack = stack;
 }
 
-const CStack* BattleStacksController::getSelectedStack()
+const CStack* BattleStacksController::getSelectedStack() const
 {
 	return selectedStack;
 }
 
-const CStack* BattleStacksController::getActiveStack()
+const CStack* BattleStacksController::getActiveStack() const
 {
 	return activeStack;
 }
 
-bool BattleStacksController::facingRight(const CStack * stack)
+bool BattleStacksController::facingRight(const CStack * stack) const
 {
-	return stackFacingRight[stack->ID];
+	return stackFacingRight.at(stack->ID);
 }
 
 bool BattleStacksController::activeStackSpellcaster()
@@ -555,7 +555,7 @@ SpellID BattleStacksController::activeStackSpellToCast()
 	return SpellID(creatureSpellToCast);
 }
 
-Point BattleStacksController::getStackPositionAtHex(BattleHex hexNum, const CStack * stack)
+Point BattleStacksController::getStackPositionAtHex(BattleHex hexNum, const CStack * stack) const
 {
 	Point ret(-500, -500); //returned value
 	if(stack && stack->initialPosition < 0) //creatures in turrets

client/battle/BattleStacksController.h

@@ -1,5 +1,5 @@
 /*
- * CBattleStacksController.h, part of VCMI engine
+ * BattleStacksController.h, part of VCMI engine
  *
  * Authors: listed in file AUTHORS in main folder
  *
@@ -31,6 +31,10 @@ class CBattleAnimation;
 class BattleRenderer;
 class IImage;
 
+/// Class responsible for handling stacks in battle
+/// Handles ordering of stacks animation
+/// As well as rendering of stacks, their amount boxes
+/// And any other effect applied to stacks
 class BattleStacksController
 {
 	BattleInterface * owner;
@@ -40,19 +44,33 @@ class BattleStacksController
 	std::shared_ptr<IImage> amountPositive;
 	std::shared_ptr<IImage> amountEffNeutral;
 
-	std::vector<CBattleAnimation *> currentAnimations; //currently displayed animations <anim, initialized>
+	/// currently displayed animations <anim, initialized>
-	std::map<int32_t, std::shared_ptr<CreatureAnimation>> stackAnimation; //animations of creatures from fighting armies (order by BattleInfo's stacks' ID)
+	std::vector<CBattleAnimation *> currentAnimations;
-	std::map<int, bool> stackFacingRight; // <creatureID, if false reverse creature's animation> //TODO: move it to battle callback
 
-	const CStack *activeStack; //number of active stack; nullptr - no one
+	/// animations of creatures from fighting armies (order by BattleInfo's stacks' ID)
-	const CStack *mouseHoveredStack; // stack below mouse pointer, used for border animation
+	std::map<int32_t, std::shared_ptr<CreatureAnimation>> stackAnimation;
-	const CStack *stackToActivate; //when animation is playing, we should wait till the end to make the next stack active; nullptr of none
-	const CStack *selectedStack; //for Teleport / Sacrifice
 
-	bool stackCanCastSpell; //if true, active stack could possibly cast some target spell
+	/// <creatureID, if false reverse creature's animation> //TODO: move it to battle callback
+	std::map<int, bool> stackFacingRight;
+
+	/// number of active stack; nullptr - no one
+	const CStack *activeStack;
+
+	/// stack below mouse pointer, used for border animation
+	const CStack *mouseHoveredStack;
+
+	///when animation is playing, we should wait till the end to make the next stack active; nullptr of none
+	const CStack *stackToActivate;
+
+	/// stack that was selected for multi-target spells - Teleport / Sacrifice
+	const CStack *selectedStack;
+
+	/// if true, active stack could possibly cast some target spell
+	bool stackCanCastSpell;
 	si32 creatureSpellToCast;
 
-	ui32 animIDhelper; //for giving IDs for animations
+	/// for giving IDs for animations
+	ui32 animIDhelper;
 
 	bool stackNeedsAmountBox(const CStack * stack);
 	void showStackAmountBox(Canvas & canvas, const CStack * stack);
@@ -63,8 +81,8 @@ class BattleStacksController
 public:
 	BattleStacksController(BattleInterface * owner);
 
-	bool shouldRotate(const CStack * stack, const BattleHex & oldPos, const BattleHex & nextHex);
+	bool shouldRotate(const CStack * stack, const BattleHex & oldPos, const BattleHex & nextHex) const;
-	bool facingRight(const CStack * stack);
+	bool facingRight(const CStack * stack) const;
 
 	void stackReset(const CStack * stack);
 	void stackAdded(const CStack * stack); //new stack appeared on battlefield
@@ -94,11 +112,11 @@ public:
 	void addNewAnim(CBattleAnimation *anim); //adds new anim to pendingAnims
 	void updateBattleAnimations();
 
-	const CStack* getActiveStack();
+	const CStack* getActiveStack() const;
-	const CStack* getSelectedStack();
+	const CStack* getSelectedStack() const;
 
 	/// returns position of animation needed to place stack in specific hex
-	Point getStackPositionAtHex(BattleHex hexNum, const CStack * creature);
+	Point getStackPositionAtHex(BattleHex hexNum, const CStack * creature) const;
 
 	friend class CBattleAnimation; // for exposing pendingAnims/creAnims/creDir to animations
 };

client/battle/CreatureAnimation.h

@@ -57,37 +57,44 @@ public:
 
 private:
 	std::string name;
+
+	/// animation for rendering stack in default orientation - facing right
 	std::shared_ptr<CAnimation> forward;
+
+	/// animation that has all its frames flipped for rendering stack facing left
 	std::shared_ptr<CAnimation> reverse;
 
 	int fullWidth;
 	int fullHeight;
 
-	// speed of animation, measure in frames per second
+	/// speed of animation, measure in frames per second
 	float speed;
 
-	// currently displayed frame. Float to allow H3-style animations where frames
+	/// currently displayed frame. Float to allow H3-style animations where frames
-	// don't display for integer number of frames
+	/// don't display for integer number of frames
 	float currentFrame;
-	// cumulative, real-time duration of animation. Used for effects like selection border
-	float elapsedTime;
-	CCreatureAnim::EAnimType type; //type of animation being displayed
 
-	// border color, disabled if alpha = 0
+	/// cumulative, real-time duration of animation. Used for effects like selection border
+	float elapsedTime;
+
+	///type of animation being displayed
+	CCreatureAnim::EAnimType type;
+
+	/// border color, disabled if alpha = 0
 	SDL_Color border;
 
 	TSpeedController speedController;
 
-	bool once; // animation will be played once and the reset to idling
+	/// animation will be played once and the reset to idling
+	bool once;
 
 	void endAnimation();
 
-
 	void genBorderPalette(IImage::BorderPallete & target);
 public:
 
-	// function(s) that will be called when animation ends, after reset to 1st frame
+	/// function(s) that will be called when animation ends, after reset to 1st frame
-	// NOTE that these function will be fired only once
+	/// NOTE that these functions will be fired only once
 	CFunctionList<void()> onAnimationReset;
 
 	int getWidth() const;
@@ -99,28 +106,35 @@ public:
 	/// in specified group of animation should be played, measured in seconds
 	CreatureAnimation(const std::string & name_, TSpeedController speedController);
 
-	void setType(CCreatureAnim::EAnimType type); //sets type of animation and cleares framecount
+	/// sets type of animation and resets framecount
-	CCreatureAnim::EAnimType getType() const; //returns type of animation
+	void setType(CCreatureAnim::EAnimType type);
+
+	/// returns currently rendered type of animation
+	CCreatureAnim::EAnimType getType() const;
 
 	void nextFrame(Canvas & canvas, bool facingRight);
 
-	// should be called every frame, return true when animation was reset to beginning
+	/// should be called every frame, return true when animation was reset to beginning
 	bool incrementFrame(float timePassed);
+
 	void setBorderColor(SDL_Color palette);
 
-	// tint color effect
+	/// apply color tint effect
 	void shiftColor(const ColorShifter * shifter);
 
-	float getCurrentFrame() const; // Gets the current frame ID relative to frame group.
+	/// Gets the current frame ID within current group.
+	float getCurrentFrame() const;
 
-	void playOnce(CCreatureAnim::EAnimType type); //plays once given stage of animation, then resets to 2
+	/// plays once given type of animation, then resets to idle
+	void playOnce(CCreatureAnim::EAnimType type);
 
-	int framesInGroup(CCreatureAnim::EAnimType group) const;
+	/// returns number of frames in selected animation type
+	int framesInGroup(CCreatureAnim::EAnimType type) const;
 
 	void pause();
 	void play();
 
-	//helpers. TODO: move them somewhere else
+	/// helpers to classify current type of animation
 	bool isDead() const;
 	bool isDying() const;
 	bool isDeadOrDying() const;