Code cleanup

client/windows/CMessage.cpp

@@ -28,7 +28,6 @@
 #include "../windows/InfoWindows.h"
 
 constexpr int RIGHT_CLICK_POPUP_MIN_SIZE = 100;
-constexpr int BEFORE_COMPONENTS = 30;
 constexpr int SIDE_MARGIN = 11;
 constexpr int TOP_MARGIN = 20;
 constexpr int BOTTOM_MARGIN = 16;
@@ -79,7 +78,7 @@ std::vector<std::string> CMessage::breakText(std::string text, size_t maxLineWid
 		ui32 wordBreak = -1; //last position for line break (last space character)
 		ui32 currPos = 0; //current position in text
 		bool opened = false; //set to true when opening brace is found
-		std::string color = ""; //color found
+		std::string color; //color found
 
 		size_t symbolSize = 0; // width of character, in bytes
 		size_t glyphWidth = 0; // width of printable glyph, pixels
@@ -118,8 +117,8 @@ std::vector<std::string> CMessage::breakText(std::string text, size_t maxLineWid
 				color = "";
 			}
 			else
-				lineWidth += (ui32)glyphWidth;
-			currPos += (ui32)symbolSize;
+				lineWidth += glyphWidth;
+			currPos += symbolSize;
 		}
 
 		// long line, create line break
@@ -128,7 +127,7 @@ std::vector<std::string> CMessage::breakText(std::string text, size_t maxLineWid
 			if(wordBreak != ui32(-1))
 				currPos = wordBreak;
 			else
-				currPos -= (ui32)symbolSize;
+				currPos -= symbolSize;
 		}
 
 		//non-blank line
@@ -188,18 +187,18 @@ std::string CMessage::guessHeader(const std::string & msg)
 int CMessage::guessHeight(const std::string & txt, int width, EFonts font)
 {
 	const auto f = graphics->fonts[font];
-	auto lines = CMessage::breakText(txt, width, font);
-	int lineHeight = static_cast<int>(f->getLineHeight());
-	return lineHeight * (int)lines.size();
+	const auto lines = CMessage::breakText(txt, width, font);
+	size_t lineHeight = f->getLineHeight();
+	return lineHeight * lines.size();
 }
 
 int CMessage::getEstimatedComponentHeight(int numComps)
 {
 	if(numComps > 8) //Bigger than 8 components - return invalid value
 		return std::numeric_limits<int>::max();
-	else if(numComps > 2)
+	if(numComps > 2)
 		return 160; // 32px * 1 row + 20 to offset
-	else if(numComps)
+	if(numComps > 0)
 		return 118; // 118 px to offset
 	return 0;
 }
@@ -210,7 +209,7 @@ void CMessage::drawIWindow(CInfoWindow * ret, std::string text, PlayerColor play
 	// game should pick smallest one that can fit text without slider
 	// or, if not possible - pick last one and use slider
 	constexpr std::array textAreaSizes = {
-//		Point(206, 72), // NOTE: this one should only be used for single-line texts
+		// FIXME: this size should only be used for single-line texts: Point(206, 72),
 		Point(270, 72),
 		Point(270, 136),
 		Point(270, 200),
@@ -230,7 +229,6 @@ void CMessage::drawIWindow(CInfoWindow * ret, std::string text, PlayerColor play
 			break; // suitable size found, use it
 	}
 
-//	int textWidth = ret->text->pos.w;
 	int textHeight = ret->text->pos.h;
 
 	if(ret->text->slider)
@@ -242,7 +240,7 @@ void CMessage::drawIWindow(CInfoWindow * ret, std::string text, PlayerColor play
 	{
 		// Compute total width of buttons
 		buttonsWidth = INTERVAL_BETWEEN_BUTTONS * (ret->buttons.size() - 1); // space between all buttons
-		for(auto & elem : ret->buttons) //and add buttons width
+		for(const auto & elem : ret->buttons) //and add buttons width
 		{
 			buttonsWidth += elem->pos.w;
 			vstd::amax(buttonsHeight, elem->pos.h);
@@ -297,7 +295,7 @@ void CMessage::drawIWindow(CInfoWindow * ret, std::string text, PlayerColor play
 			int buttonPosX = ret->pos.w / 2 - buttonsWidth / 2;
 			int buttonPosY = ret->pos.h - BOTTOM_MARGIN - ret->buttons[0]->pos.h;
 
-			for(auto & elem : ret->buttons)
+			for(const auto & elem : ret->buttons)
 			{
 				elem->moveBy(Point(buttonPosX, buttonPosY));
 				buttonPosX += elem->pos.w + INTERVAL_BETWEEN_BUTTONS;

client/windows/InfoWindows.cpp

@@ -55,7 +55,7 @@ CSelWindow::CSelWindow( const std::string & Text, PlayerColor player, int charpe
 
 	if(buttons.size() > 1 && askID.getNum() >= 0) //cancel button functionality
 	{
-		buttons.back()->addCallback([askID](){LOCPLINT->cb.get()->selectionMade(0, askID);});
+		buttons.back()->addCallback([askID](){LOCPLINT->cb->selectionMade(0, askID);});
 		//buttons.back()->addCallback(std::bind(&CCallback::selectionMade, LOCPLINT->cb.get(), 0, askID));
 	}
 
@@ -91,16 +91,16 @@ void CSelWindow::madeChoiceAndClose()
 	close();
 }
 
-CInfoWindow::CInfoWindow(std::string Text, PlayerColor player, const TCompsInfo & comps, const TButtonsInfo & Buttons)
+CInfoWindow::CInfoWindow(const std::string & Text, PlayerColor player, const TCompsInfo & comps, const TButtonsInfo & Buttons)
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255 - DISPOSE);
 
 	backgroundTexture = std::make_shared<CFilledTexture>(ImagePath::builtin("DiBoxBck"), pos);
 
 	ID = QueryID(-1);
-	for(auto & Button : Buttons)
+	for(const auto & Button : Buttons)
 	{
-		std::shared_ptr<CButton> button = std::make_shared<CButton>(Point(0, 0), Button.first, CButton::tooltip(), std::bind(&CInfoWindow::close, this));
+		auto button = std::make_shared<CButton>(Point(0, 0), Button.first, CButton::tooltip(), std::bind(&CInfoWindow::close, this));
 		button->setBorderColor(Colors::METALLIC_GOLD);
 		button->addCallback(Button.second); //each button will close the window apart from call-defined actions
 		buttons.push_back(button);
@@ -159,9 +159,9 @@ void CInfoWindow::showYesNoDialog(const std::string & text, const TCompsInfo & c
 {
 	assert(!LOCPLINT || LOCPLINT->showingDialog->get());
 	std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
-	pom.push_back({AnimationPath::builtin("IOKAY.DEF"), 0});
-	pom.push_back({AnimationPath::builtin("ICANCEL.DEF"), 0});
-	std::shared_ptr<CInfoWindow> temp = std::make_shared<CInfoWindow>(text, player, components, pom);
+	pom.emplace_back(AnimationPath::builtin("IOKAY.DEF"), nullptr);
+	pom.emplace_back(AnimationPath::builtin("ICANCEL.DEF"), nullptr);
+	auto temp = std::make_shared<CInfoWindow>(text, player, components, pom);
 
 	temp->buttons[0]->addCallback(onYes);
 	temp->buttons[1]->addCallback(onNo);
@@ -172,11 +172,11 @@ void CInfoWindow::showYesNoDialog(const std::string & text, const TCompsInfo & c
 std::shared_ptr<CInfoWindow> CInfoWindow::create(const std::string & text, PlayerColor playerID, const TCompsInfo & components)
 {
 	std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
-	pom.push_back({AnimationPath::builtin("IOKAY.DEF"), 0});
+	pom.emplace_back(AnimationPath::builtin("IOKAY.DEF"), nullptr);
 	return std::make_shared<CInfoWindow>(text, playerID, components, pom);
 }
 
-std::string CInfoWindow::genText(std::string title, std::string description)
+std::string CInfoWindow::genText(const std::string & title, const std::string & description)
 {
 	return std::string("{") + title + "}" + "\n\n" + description;
 }
@@ -207,7 +207,7 @@ void CRClickPopup::createAndPush(const std::string & txt, const CInfoWindow::TCo
 	GH.windows().createAndPushWindow<CRClickPopupInt>(temp);
 }
 
-void CRClickPopup::createAndPush(const std::string & txt, std::shared_ptr<CComponent> component)
+void CRClickPopup::createAndPush(const std::string & txt, const std::shared_ptr<CComponent> & component)
 {
 	CInfoWindow::TCompsInfo intComps;
 	intComps.push_back(component);
@@ -244,7 +244,7 @@ void CRClickPopup::createAndPush(const CGObjectInstance * obj, const Point & p,
 	}
 }
 
-CRClickPopupInt::CRClickPopupInt(std::shared_ptr<CIntObject> our)
+CRClickPopupInt::CRClickPopupInt(const std::shared_ptr<CIntObject> & our)
 {
 	CCS->curh->hide();
 	defActions = SHOWALL | UPDATE;

client/windows/InfoWindows.h

@@ -47,7 +47,7 @@ public:
 
 	void sliderMoved(int to);
 
-	CInfoWindow(std::string Text, PlayerColor player, const TCompsInfo & comps = TCompsInfo(), const TButtonsInfo & Buttons = TButtonsInfo());
+	CInfoWindow(const std::string & Text, PlayerColor player, const TCompsInfo & comps = TCompsInfo(), const TButtonsInfo & Buttons = TButtonsInfo());
 	CInfoWindow();
 	~CInfoWindow();
 
@@ -57,7 +57,7 @@ public:
 	static std::shared_ptr<CInfoWindow> create(const std::string & text, PlayerColor playerID = PlayerColor(1), const TCompsInfo & components = TCompsInfo());
 
 	/// create text from title and description: {title}\n\n description
-	static std::string genText(std::string title, std::string description);
+	static std::string genText(const std::string & title, const std::string & description);
 };
 
 /// popup displayed on R-click
@@ -69,7 +69,7 @@ public:
 
 	static std::shared_ptr<WindowBase> createCustomInfoWindow(Point position, const CGObjectInstance * specific);
 	static void createAndPush(const std::string & txt, const CInfoWindow::TCompsInfo & comps = CInfoWindow::TCompsInfo());
-	static void createAndPush(const std::string & txt, std::shared_ptr<CComponent> component);
+	static void createAndPush(const std::string & txt, const std::shared_ptr<CComponent> & component);
 	static void createAndPush(const CGObjectInstance * obj, const Point & p, ETextAlignment alignment = ETextAlignment::BOTTOMRIGHT);
 };
 
@@ -79,8 +79,8 @@ class CRClickPopupInt : public CRClickPopup
 	std::shared_ptr<CIntObject> inner;
 
 public:
-	CRClickPopupInt(std::shared_ptr<CIntObject> our);
-	virtual ~CRClickPopupInt();
+	CRClickPopupInt(const std::shared_ptr<CIntObject> & our);
+	~CRClickPopupInt();
 };
 
 /// popup on adventure map for town\hero and other objects with customized popup content