Garrison: Refactor Left Click

Restructure large nested if statements into more shallow blocks
Factor out contents of if statements to functions for better readability
    * viewInfo
    * hightlightOrDropArtifact
    * split

client/widgets/CGarrisonInt.cpp

@@ -144,141 +144,142 @@ void CGarrisonSlot::clickRight(tribool down, bool previousState)
 		GH.pushInt(new CStackWindow(myStack, true));
 	}
 }
-void CGarrisonSlot::clickLeft(tribool down, bool previousState)
+
+bool CGarrisonSlot::viewInfo()
 {
-	if(down)
-	{
-		bool refr = false;
-		const CGarrisonSlot * selection = owner->getSelection();
-		if(selection)
-		{
-			if(selection == this) //view info
-			{
-				UpgradeInfo pom;
-				LOCPLINT->cb->getUpgradeInfo(getObj(), ID, pom);
+	UpgradeInfo pom;
+	LOCPLINT->cb->getUpgradeInfo(getObj(), ID, pom);
 
-				bool canUpgrade = getObj()->tempOwner == LOCPLINT->playerID && pom.oldID>=0; //upgrade is possible
-				bool canDismiss = getObj()->tempOwner == LOCPLINT->playerID && (getObj()->stacksCount()>1  || !getObj()->needsLastStack());
-				std::function<void(CreatureID)> upgr = nullptr;
-				std::function<void()> dism = nullptr;
-				if(canUpgrade) upgr = [=] (CreatureID newID) { LOCPLINT->cb->upgradeCreature(getObj(), ID, newID); };
-				if(canDismiss) dism = [=] { LOCPLINT->cb->dismissCreature(getObj(), ID); };
+	bool canUpgrade = getObj()->tempOwner == LOCPLINT->playerID && pom.oldID>=0; //upgrade is possible
+	bool canDismiss = getObj()->tempOwner == LOCPLINT->playerID && (getObj()->stacksCount()>1  || !getObj()->needsLastStack());
+	std::function<void(CreatureID)> upgr = nullptr;
+	std::function<void()> dism = nullptr;
+	if(canUpgrade) upgr = [=] (CreatureID newID) { LOCPLINT->cb->upgradeCreature(getObj(), ID, newID); };
+	if(canDismiss) dism = [=] { LOCPLINT->cb->dismissCreature(getObj(), ID); };
 
-				owner->selectSlot(nullptr);
-				owner->setSplittingMode(false);
+	owner->selectSlot(nullptr);
+	owner->setSplittingMode(false);
 
-				for(auto & elem : owner->splitButtons)
-					elem->block(true);
+	for(auto & elem : owner->splitButtons)
+		elem->block(true);
 
-				redraw();
-				refr = true;
-				GH.pushInt(new CStackWindow(myStack, dism, pom, upgr));
-			}
-			else
+	redraw();
+	GH.pushInt(new CStackWindow(myStack, dism, pom, upgr));
+	return true;
+}
+
+bool CGarrisonSlot::highlightOrDropArtifact()
+{
+	bool artSelected = false;
+	if (CWindowWithArtifacts* chw = dynamic_cast<CWindowWithArtifacts*>(GH.topInt())) //dirty solution
+	{
+		const CArtifactsOfHero::SCommonPart *commonInfo = chw->artSets.front()->commonInfo;
+		if (const CArtifactInstance *art = commonInfo->src.art)
+		{
+			const CGHeroInstance *srcHero = commonInfo->src.AOH->getHero();
+			artSelected = true;
+			if (myStack) // try dropping the artifact only if the slot isn't empty
 			{
-				// Only allow certain moves if troops aren't removable or not ours.
-				if (  ( selection->our()//our creature is selected
-				     || selection->creature == creature )//or we are rebalancing army
-				   && ( owner->removableUnits
-				     || (upg == 0 &&  ( selection->upg == 1 && !creature ) )
-					 || (upg == 1 &&    selection->upg == 1 ) ) )
-				{
-					//we want to split
-					if((owner->getSplittingMode() || LOCPLINT->shiftPressed())
-						&& (!creature
-							|| (creature == selection->creature)))
-					{
-						owner->p2 = ID; //store the second stack pos
-						owner->pb = upg;//store the second stack owner (up or down army)
-						owner->setSplittingMode(false);
-
-						int minLeft=0, minRight=0;
-
-						if(upg != selection->upg) //not splitting within same army
-						{
-							if(selection->getObj()->stacksCount() == 1 //we're splitting away the last stack
-								&& selection->getObj()->needsLastStack() )
-							{
-								minLeft = 1;
-							}
-							if(getObj()->stacksCount() == 1 //destination army can't be emptied, unless we're rebalancing two stacks of same creature
-								&& selection->creature == creature
-								&& getObj()->needsLastStack() )
-							{
-								minRight = 1;
-							}
-						}
-
-						int countLeft = selection->myStack ? selection->myStack->count : 0;
-						int countRight = myStack ? myStack->count : 0;
-
-						GH.pushInt(new CSplitWindow(selection->creature, std::bind(&CGarrisonInt::splitStacks, owner, _1, _2),
-						                            minLeft, minRight, countLeft, countRight));
-						refr = true;
-					}
-					else if(creature != selection->creature) //swap
+				ArtifactLocation src(srcHero, commonInfo->src.slotID);
+				ArtifactLocation dst(myStack, ArtifactPosition::CREATURE_SLOT);
+				if (art->canBePutAt(dst, true))
+				{	//equip clicked stack
+					if(dst.getArt())
 					{
-						LOCPLINT->cb->swapCreatures(
-							(!upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
-							(!selection->upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
-							ID,selection->ID);
+						//creature can wear only one active artifact
+						//if we are placing a new one, the old one will be returned to the hero's backpack
+						LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero, dst.getArt()->firstBackpackSlot(srcHero)));
 					}
-					else //merge
-					{
-						LOCPLINT->cb->mergeStacks(
-							(!selection->upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
-							(!upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
-							selection->ID,ID);
-					}
-				}
-				else // Highlight
-				{
-					if(creature)
-						owner->selectSlot(this);
-					redraw();
-					refr = true;
+					LOCPLINT->cb->swapArtifacts(src, dst);
 				}
 			}
 		}
-		else //highlight or drop artifact
+	}
+	if (!artSelected && creature)
+	{
+		owner->selectSlot(this);
+		if(creature)
 		{
-			bool artSelected = false;
-			if (CWindowWithArtifacts* chw = dynamic_cast<CWindowWithArtifacts*>(GH.topInt())) //dirty solution
-			{
-				const CArtifactsOfHero::SCommonPart *commonInfo = chw->artSets.front()->commonInfo;
-				if (const CArtifactInstance *art = commonInfo->src.art)
-				{
-					const CGHeroInstance *srcHero = commonInfo->src.AOH->getHero();
-					artSelected = true;
-					if (myStack) // try dropping the artifact only if the slot isn't empty
-					{
-						ArtifactLocation src(srcHero, commonInfo->src.slotID);
-						ArtifactLocation dst(myStack, ArtifactPosition::CREATURE_SLOT);
-						if (art->canBePutAt(dst, true))
-						{	//equip clicked stack
-							if(dst.getArt())
-							{
-								//creature can wear only one active artifact
-								//if we are placing a new one, the old one will be returned to the hero's backpack
-								LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero, dst.getArt()->firstBackpackSlot(srcHero)));
-							}
-							LOCPLINT->cb->swapArtifacts(src, dst);
-						}
-					}
-				}
-			}
-			if (!artSelected && creature)
-			{
+			for(auto & elem : owner->splitButtons)
+				elem->block(!our());
+		}
+	}
+	redraw();
+	return true;
+}
+
+bool CGarrisonSlot::split()
+{
+	const CGarrisonSlot * selection = owner->getSelection();
+	owner->p2 = ID;   // store the second stack pos
+	owner->pb = upg;  // store the second stack owner (up or down army)
+	owner->setSplittingMode(false);
+
+	int minLeft=0, minRight=0;
+
+	if(upg != selection->upg) // not splitting within same army
+	{
+		if(selection->getObj()->stacksCount() == 1 // we're splitting away the last stack
+			&& selection->getObj()->needsLastStack() )
+		{
+			minLeft = 1;
+		}
+		// destination army can't be emptied, unless we're rebalancing two stacks of same creature
+		if(getObj()->stacksCount() == 1
+			&& selection->creature == creature
+			&& getObj()->needsLastStack() )
+		{
+			minRight = 1;
+		}
+	}
+
+	int countLeft = selection->myStack ? selection->myStack->count : 0;
+	int countRight = myStack ? myStack->count : 0;
+
+	GH.pushInt(new CSplitWindow(selection->creature, std::bind(&CGarrisonInt::splitStacks, owner, _1, _2),
+	                            minLeft, minRight, countLeft, countRight));
+	return true;
+}
+
+void CGarrisonSlot::clickLeft(tribool down, bool previousState)
+{
+	if(down)
+	{
+		bool refr = false;
+		const CGarrisonSlot * selection = owner->getSelection();
+		if(!selection)
+			refr = highlightOrDropArtifact();
+		else if(selection == this)
+			refr = viewInfo();
+		// Only allow certain moves if troops aren't removable or not ours.
+		else if (!(  ( selection->our()//our creature is selected
+		         || selection->creature == creature )//or we are rebalancing army
+		       && ( owner->removableUnits
+		         || (upg == 0 &&  ( selection->upg == 1 && !creature ) )
+		         || (upg == 1 &&    selection->upg == 1 ) ) ))
+		{
+			// Highlight
+			if(creature)
 				owner->selectSlot(this);
-				if(creature)
-				{
-					for(auto & elem : owner->splitButtons)
-						elem->block(!our());
-				}
-			}
 			redraw();
 			refr = true;
 		}
+		// we want to split
+		else if(  (owner->getSplittingMode() || LOCPLINT->shiftPressed())
+		       && (!creature || creature == selection->creature) )
+			refr = split();
+		// swap
+		else if(creature != selection->creature)
+			LOCPLINT->cb->swapCreatures(
+				(!upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
+				(!selection->upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
+				ID,selection->ID);
+		// merge
+		else
+			LOCPLINT->cb->mergeStacks(
+				(!selection->upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
+				(!upg)?(owner->armedObjs[0]):(owner->armedObjs[1]),
+				selection->ID,ID);
 		if(refr) {hover(false);	hover(true); } //to refresh statusbar
 	}
 }

client/widgets/CGarrisonInt.h

@@ -34,6 +34,10 @@ class CGarrisonSlot : public CIntObject
 	CAnimImage * selectionImage; // image for selection, not always visible
 	CLabel * stackCount;
 
+	bool viewInfo();
+	bool highlightOrDropArtifact();
+	bool split();
+
 	void setHighlight(bool on);
 public:
 	virtual void hover (bool on); //call-in