Code cleanup

client/widgets/Buttons.cpp

@@ -74,7 +74,7 @@ void ButtonBase::setTextOverlay(const std::string & Text, EFonts font, ColorRGBA
 	update();
 }
 
-void ButtonBase::setOverlay(std::shared_ptr<CIntObject> newOverlay)
+void ButtonBase::setOverlay(const std::shared_ptr<CIntObject>& newOverlay)
 {
 	overlay = newOverlay;
 	if(overlay)
@@ -133,7 +133,7 @@ void ButtonBase::setConfigurable(const JsonPath & jsonName, bool playerColoredBu
 		image->playerColored(LOCPLINT->playerID);
 }
 
-void CButton::addHoverText(EButtonState state, std::string text)
+void CButton::addHoverText(EButtonState state, const std::string & text)
 {
 	hoverTexts[vstd::to_underlying(state)] = text;
 }
@@ -278,7 +278,7 @@ void CButton::clickCancel(const Point & cursorPosition)
 
 void CButton::showPopupWindow(const Point & cursorPosition)
 {
-	if(helpBox.size()) //there is no point to show window with nothing inside...
+	if(!helpBox.empty()) //there is no point to show window with nothing inside...
 		CRClickPopup::createAndPush(helpBox);
 }
 
@@ -333,14 +333,15 @@ ButtonBase::~ButtonBase() = default;
 
 CButton::CButton(Point position, const AnimationPath &defName, const std::pair<std::string, std::string> &help, CFunctionList<void()> Callback, EShortcut key, bool playerColoredButton):
 	ButtonBase(position, defName, key, playerColoredButton),
-    callback(Callback)
+	callback(Callback),
+	helpBox(help.second),
+	hoverable(false),
+	actOnDown(false),
+	soundDisabled(false)
 {
 	defActions = 255-DISPOSE;
 	addUsedEvents(LCLICK | SHOW_POPUP | HOVER | KEYBOARD);
-
-	hoverable = actOnDown = soundDisabled = false;
 	hoverTexts[0] = help.first;
-	helpBox=help.second;
 }
 
 void ButtonBase::setPlayerColor(PlayerColor player)
@@ -413,14 +414,12 @@ bool CToggleBase::isSelected() const
 	return selected;
 }
 
-bool CToggleBase::canActivate()
+bool CToggleBase::canActivate() const
 {
-	if (selected && !allowDeselection)
+	return !selected || allowDeselection;
-		return false;
-	return true;
 }
 
-void CToggleBase::addCallback(std::function<void(bool)> function)
+void CToggleBase::addCallback(const std::function<void(bool)> & function)
 {
 	callback += function;
 }
@@ -501,7 +500,7 @@ void CToggleButton::clickCancel(const Point & cursorPosition)
 	doSelect(isSelected());
 }
 
-void CToggleGroup::addCallback(std::function<void(int)> callback)
+void CToggleGroup::addCallback(const std::function<void(int)> & callback)
 {
 	onChange += callback;
 }
@@ -511,7 +510,7 @@ void CToggleGroup::resetCallback()
 	onChange.clear();
 }
 
-void CToggleGroup::addToggle(int identifier, std::shared_ptr<CToggleBase> button)
+void CToggleGroup::addToggle(int identifier, const std::shared_ptr<CToggleBase> & button)
 {
 	if(auto intObj = std::dynamic_pointer_cast<CIntObject>(button)) // hack-ish workagound to avoid diamond problem with inheritance
 	{
@@ -538,11 +537,8 @@ void CToggleGroup::setSelected(int id)
 
 void CToggleGroup::setSelectedOnly(int id)
 {
-	for(auto it = buttons.begin(); it != buttons.end(); it++)
+	for(const auto & button : buttons)
-	{
+		button.second->setEnabled(button.first == id);
-		int buttonId = it->first;
-		buttons[buttonId]->setEnabled(buttonId == id);
-	}
 
 	selectionChanged(id);
 }

client/widgets/Buttons.h

@@ -59,7 +59,7 @@ public:
 	void setImageOrder(int state1, int state2, int state3, int state4);
 
 	/// adds overlay on top of button image. Only one overlay can be active at once
-	void setOverlay(std::shared_ptr<CIntObject> newOverlay);
+	void setOverlay(const std::shared_ptr<CIntObject>& newOverlay);
 	void setTextOverlay(const std::string & Text, EFonts font, ColorRGBA color);
 };
 
@@ -90,7 +90,7 @@ public:
 	/// adds one more callback to on-click actions
 	void addCallback(const std::function<void()> & callback);
 
-	void addHoverText(EButtonState state, std::string text);
+	void addHoverText(EButtonState state, const std::string & text);
 
 	void block(bool on);
 
@@ -134,7 +134,7 @@ protected:
 	virtual void doSelect(bool on);
 
 	// returns true if toggle can change its state
-	bool canActivate();
+	bool canActivate() const;
 
 public:
 	CToggleBase(CFunctionList<void(bool)> callback);
@@ -150,7 +150,7 @@ public:
 
 	void setAllowDeselection(bool on);
 
-	void addCallback(std::function<void(bool)> callback);
+	void addCallback(const std::function<void(bool)> & callback);
 
 	/// Set whether the toggle is currently enabled for user to use, this is only inplemented in ToggleButton, not for other toggles yet.
 	virtual void setEnabled(bool enabled);
@@ -186,11 +186,11 @@ public:
 
 	CToggleGroup(const CFunctionList<void(int)> & OnChange);
 
-	void addCallback(std::function<void(int)> callback);
+	void addCallback(const std::function<void(int)> & callback);
 	void resetCallback();
 
 	/// add one toggle/button into group
-	void addToggle(int index, std::shared_ptr<CToggleBase> button);
+	void addToggle(int index, const std::shared_ptr<CToggleBase> & button);
 	/// Changes selection to specific value. Will select toggle with this ID, if present
 	void setSelected(int id);
 	/// in some cases, e.g. LoadGame difficulty selection, after refreshing the UI, the ToggleGroup should 

client/windows/CCastleInterface.cpp

@@ -1332,11 +1332,11 @@ void CCastleInterface::recreateIcons()
 	hall = std::make_shared<CTownInfo>(80, 413, town, true);
 	fort = std::make_shared<CTownInfo>(122, 413, town, false);
 
-	fastTownHall = std::make_shared<CButton>(Point(80, 413), AnimationPath::builtin("castleInterfaceQuickAccess"), CButton::tooltip(), [&](){ builds->enterTownHall(); });
+	fastTownHall = std::make_shared<CButton>(Point(80, 413), AnimationPath::builtin("castleInterfaceQuickAccess"), CButton::tooltip(), [this](){ builds->enterTownHall(); });
 	fastTownHall->setOverlay(std::make_shared<CAnimImage>(AnimationPath::builtin("ITMTL"), town->hallLevel()));
 
 	int imageIndex = town->fortLevel() == CGTownInstance::EFortLevel::NONE ? 3 : town->fortLevel() - 1;
-	fastArmyPurchase = std::make_shared<CButton>(Point(122, 413), AnimationPath::builtin("castleInterfaceQuickAccess"), CButton::tooltip(), [&](){ builds->enterToTheQuickRecruitmentWindow(); });
+	fastArmyPurchase = std::make_shared<CButton>(Point(122, 413), AnimationPath::builtin("castleInterfaceQuickAccess"), CButton::tooltip(), [this](){ builds->enterToTheQuickRecruitmentWindow(); });
 	fastArmyPurchase->setOverlay(std::make_shared<CAnimImage>(AnimationPath::builtin("itmcl"), imageIndex));
 
 	fastMarket = std::make_shared<LRClickableArea>(Rect(163, 410, 64, 42), [&]()

client/windows/CKingdomInterface.cpp

@@ -821,11 +821,11 @@ CTownItem::CTownItem(const CGTownInstance * Town)
 		available.push_back(std::make_shared<CCreaInfo>(Point(48+37*(int)i, 78), town, (int)i, true, false));
 	}
 
-	fastTownHall = std::make_shared<CButton>(Point(69, 31), AnimationPath::builtin("castleInterfaceQuickAccessz"), CButton::tooltip(), [&]() { std::make_shared<CCastleBuildings>(town)->enterTownHall(); });
+	fastTownHall = std::make_shared<CButton>(Point(69, 31), AnimationPath::builtin("castleInterfaceQuickAccessz"), CButton::tooltip(), [this]() { std::make_shared<CCastleBuildings>(town)->enterTownHall(); });
 	fastTownHall->setOverlay(std::make_shared<CAnimImage>(AnimationPath::builtin("ITMTL"), town->hallLevel()));
 
 	int imageIndex = town->fortLevel() == CGTownInstance::EFortLevel::NONE ? 3 : town->fortLevel() - 1;
-	fastArmyPurchase = std::make_shared<CButton>(Point(111, 31), AnimationPath::builtin("castleInterfaceQuickAccessz"), CButton::tooltip(), [&]() { std::make_shared<CCastleBuildings>(town)->enterToTheQuickRecruitmentWindow(); });
+	fastArmyPurchase = std::make_shared<CButton>(Point(111, 31), AnimationPath::builtin("castleInterfaceQuickAccessz"), CButton::tooltip(), [this]() { std::make_shared<CCastleBuildings>(town)->enterToTheQuickRecruitmentWindow(); });
 	fastArmyPurchase->setOverlay(std::make_shared<CAnimImage>(AnimationPath::builtin("itmcl"), imageIndex));
 
 	fastTavern = std::make_shared<LRClickableArea>(Rect(5, 6, 58, 64), [&]()