Changes to rest of the code according to review:

- renamed status bar method clearMatching -> clearIfMatching
- renamed class ColorShifterAddMul -> ColorShifterMultiplyAndAdd
- fixed missing return from function
- fixed potential access to deleted object

client/battle/BattleActionsController.cpp

@@ -668,7 +668,7 @@ void BattleActionsController::handleHex(BattleHex myNumber, int eventType)
 				CCS->curh->changeGraphic(cursorType, cursorFrame);
 
 			if (!currentConsoleMsg.empty())
-				owner->controlPanel->console->clearMatching(currentConsoleMsg);
+				owner->controlPanel->console->clearIfMatching(currentConsoleMsg);
 			if (!newConsoleMsg.empty())
 				owner->controlPanel->console->write(newConsoleMsg);
 

client/battle/BattleInterfaceClasses.cpp

@@ -106,7 +106,7 @@ BattleConsole::BattleConsole(const Rect & position) : lastShown(-1)
 	pos.h = position.h;
 }
 
-void BattleConsole::clearMatching(const std::string & Text)
+void BattleConsole::clearIfMatching(const std::string & Text)
 {
 	if (ingcAlter == Text)
 		clear();

client/battle/BattleInterfaceClasses.h

@@ -55,7 +55,7 @@ public:
 	void scrollUp(ui32 by = 1); //scrolls console up by 'by' positions
 	void scrollDown(ui32 by = 1); //scrolls console up by 'by' positions
 
-	void clearMatching(const std::string & Text) override;
+	void clearIfMatching(const std::string & Text) override;
 	void clear() override;
 	void write(const std::string & Text) override;
 	void lock(bool shouldLock) override;

client/battle/BattleStacksController.cpp

@@ -76,10 +76,10 @@ BattleStacksController::BattleStacksController(BattleInterface * owner):
 	amountNegative   = IImage::createFromFile("CMNUMWIN.BMP");
 	amountEffNeutral = IImage::createFromFile("CMNUMWIN.BMP");
 
-	ColorShifterAddMulExcept shifterNormal  ({0,0,0,0}, {150,  50, 255, 255}, {255, 231, 132, 255});
+	static const ColorShifterMultiplyAndAddExcept shifterNormal  ({150,  50, 255, 255}, {0,0,0,0}, {255, 231, 132, 255});
-	ColorShifterAddMulExcept shifterPositive({0,0,0,0}, { 50, 255,  50, 255}, {255, 231, 132, 255});
+	static const ColorShifterMultiplyAndAddExcept shifterPositive({ 50, 255,  50, 255}, {0,0,0,0}, {255, 231, 132, 255});
-	ColorShifterAddMulExcept shifterNegative({0,0,0,0}, {255,  50,  50, 255}, {255, 231, 132, 255});
+	static const ColorShifterMultiplyAndAddExcept shifterNegative({255,  50,  50, 255}, {0,0,0,0}, {255, 231, 132, 255});
-	ColorShifterAddMulExcept shifterNeutral ({0,0,0,0}, {255, 255,  50, 255}, {255, 231, 132, 255});
+	static const ColorShifterMultiplyAndAddExcept shifterNeutral ({255, 255,  50, 255}, {0,0,0,0}, {255, 231, 132, 255});
 
 	amountNormal->adjustPalette(&shifterNormal);
 	amountPositive->adjustPalette(&shifterPositive);
@@ -160,10 +160,11 @@ void BattleStacksController::stackReset(const CStack * stack)
 	if(stack->alive() && animation->isDeadOrDying())
 		animation->setType(CCreatureAnim::HOLDING);
 
+	static const ColorShifterMultiplyAndAdd shifterClone ({255, 255, 0, 255}, {0, 0, 255, 0});
+
 	if (stack->isClone())
 	{
-		auto shifter = ColorShifterAddMul::deepBlue();
+		animation->shiftColor(&shifterClone);
-		animation->shiftColor(&shifter);
 	}
 
 	//TODO: handle more cases

client/gui/CIntObject.h

@@ -221,7 +221,7 @@ class IStatusBar
 public:
 	virtual ~IStatusBar();
 	virtual void clear() = 0;
-	virtual void clearMatching(const std::string & Text) = 0;
+	virtual void clearIfMatching(const std::string & Text) = 0;
 	virtual void write(const std::string & Text) = 0;
 	virtual void lock(bool shouldLock) = 0;
 };

client/gui/SDL_Extensions.h

@@ -158,6 +158,7 @@ struct ColorPutter
 
 typedef void (*BlitterWithRotationVal)(SDL_Surface *src,SDL_Rect srcRect, SDL_Surface * dst, SDL_Rect dstRect, ui8 rotation);
 
+/// Base class for applying palette transformation on images
 class ColorShifter
 {
 public:
@@ -165,39 +166,38 @@ public:
 	virtual SDL_Color shiftColor(SDL_Color input) const = 0;
 };
 
-class ColorShifterAddMul : public ColorShifter
+/// Generic class for palette transformation
+/// formula:
+/// result = input * factor + added
+class ColorShifterMultiplyAndAdd : public ColorShifter
 {
-	SDL_Color add;
+	SDL_Color added;
-	SDL_Color mul;
+	SDL_Color factor;
+
 public:
-
+	ColorShifterMultiplyAndAdd(SDL_Color factor, SDL_Color added) :
-	static ColorShifterAddMul deepBlue()
+		factor(factor),
-	{
+		added(added)
-		return ColorShifterAddMul({0, 0, 255, 0}, {255, 255, 0, 255});
-	}
-
-	ColorShifterAddMul(SDL_Color add, SDL_Color mul) :
-		add(add),
-		mul(mul)
 	{}
 
 	SDL_Color shiftColor(SDL_Color input) const override
 	{
 		return {
-			uint8_t(std::min(255.f, std::round(input.r * float(mul.r) / 255 + add.r))),
+			uint8_t(std::min(255.f, std::round(input.r * float(factor.r) / 255.f + added.r))),
-			uint8_t(std::min(255.f, std::round(input.g * float(mul.g) / 255 + add.g))),
+			uint8_t(std::min(255.f, std::round(input.g * float(factor.g) / 255.f + added.g))),
-			uint8_t(std::min(255.f, std::round(input.b * float(mul.b) / 255 + add.b))),
+			uint8_t(std::min(255.f, std::round(input.b * float(factor.b) / 255.f + added.b))),
-			uint8_t(std::min(255.f, std::round(input.a * float(mul.a) / 255 + add.a))),
+			uint8_t(std::min(255.f, std::round(input.a * float(factor.a) / 255.f + added.a))),
 		};
 	}
 };
 
-class ColorShifterAddMulExcept : public ColorShifterAddMul
+/// Color shifter that allows to specify color to be excempt from changes
+class ColorShifterMultiplyAndAddExcept : public ColorShifterMultiplyAndAdd
 {
 	SDL_Color ignored;
 public:
-	ColorShifterAddMulExcept(SDL_Color add, SDL_Color mul, SDL_Color ignored) :
+	ColorShifterMultiplyAndAddExcept(SDL_Color factor, SDL_Color added, SDL_Color ignored) :
-		ColorShifterAddMul(add, mul),
+		ColorShifterMultiplyAndAdd(factor, added),
 		ignored(ignored)
 	{}
 
@@ -205,7 +205,7 @@ public:
 	{
 		if ( input.r == ignored.r && input.g == ignored.g && input.b == ignored.b && input.a == ignored.a)
 			return input;
-		return ColorShifterAddMul::shiftColor(input);
+		return ColorShifterMultiplyAndAdd::shiftColor(input);
 	}
 };
 

client/mainmenu/CMainMenu.cpp

@@ -369,11 +369,11 @@ CMultiMode::CMultiMode(ESelectionScreen ScreenType)
 	: screenType(ScreenType)
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL_NO_DISPOSE;
-	background = std::make_shared<CPicture>("MUPOPUP.bmp");
-	background->convertToScreenBPP(); //so we could draw without problems
-	blitAt(CPicture("MUMAP.bmp").getSurface(), 16, 77, background->getSurface());
 	pos = background->center(); //center, window has size of bg graphic
 
+	background = std::make_shared<CPicture>("MUPOPUP.bmp");
+	picture    = std::make_shared<CPicture>("MUMAP.bmp", 16, 77);
+
 	statusBar = CGStatusBar::create(std::make_shared<CPicture>(background->getSurface(), Rect(7, 465, 440, 18), 7, 465));
 	playerName = std::make_shared<CTextInput>(Rect(19, 436, 334, 16), background->getSurface());
 	playerName->setText(settings["general"]["playerName"].String());

client/mainmenu/CMainMenu.h

@@ -80,6 +80,7 @@ class CMultiMode : public WindowBase
 public:
 	ESelectionScreen screenType;
 	std::shared_ptr<CPicture> background;
+	std::shared_ptr<CPicture> picture;
 	std::shared_ptr<CTextInput> playerName;
 	std::shared_ptr<CButton> buttonHotseat;
 	std::shared_ptr<CButton> buttonHost;

client/widgets/AdventureMapClasses.cpp

@@ -1126,7 +1126,7 @@ void CInGameConsole::textEdited(const SDL_TextEditingEvent & event)
 
 void CInGameConsole::startEnteringText()
 {
-	auto * statusBar = dynamic_cast<CGStatusBar*>(GH.statusbar.get());
+	auto statusBar = std::dynamic_pointer_cast<CGStatusBar>(GH.statusbar);
 
 	if (statusBar)
 	{
@@ -1158,7 +1158,7 @@ void CInGameConsole::endEnteringText(bool printEnteredText)
 	}
 	enteredText.clear();
 
-	auto * statusBar = dynamic_cast<CGStatusBar*>(GH.statusbar.get());
+	auto statusBar = std::dynamic_pointer_cast<CGStatusBar>(GH.statusbar);
 
 	if(statusBar)
 	{

client/widgets/Buttons.cpp

@@ -210,7 +210,7 @@ void CButton::hover (bool on)
 		if (on)
 			GH.statusbar->write(name);
 		else
-			GH.statusbar->clearMatching(name);
+			GH.statusbar->clearIfMatching(name);
 	}
 }
 

client/widgets/MiscWidgets.cpp

@@ -35,7 +35,7 @@ void CHoverableArea::hover (bool on)
 	if (on)
 		GH.statusbar->write(hoverText);
 	else
-		GH.statusbar->clearMatching(hoverText);
+		GH.statusbar->clearIfMatching(hoverText);
 }
 
 CHoverableArea::CHoverableArea()

client/widgets/TextControls.cpp

@@ -345,7 +345,7 @@ void CGStatusBar::write(const std::string & Text)
 		CLabel::setText(Text);
 }
 
-void CGStatusBar::clearMatching(const std::string & Text)
+void CGStatusBar::clearIfMatching(const std::string & Text)
 {
 	if (getText() == Text)
 		clear();

client/widgets/TextControls.h

@@ -138,7 +138,7 @@ public:
 		return ret;
 	}
 
-	void clearMatching(const std::string & Text) override;
+	void clearIfMatching(const std::string & Text) override;
 	void clear() override;//clears statusbar and refreshes
 	void write(const std::string & Text) override; //prints text and refreshes statusbar
 

client/windows/CCastleInterface.cpp

@@ -1035,7 +1035,7 @@ void CCreaInfo::hover(bool on)
 	}
 	else
 	{
-		GH.statusbar->clearMatching(message);
+		GH.statusbar->clearIfMatching(message);
 	}
 }
 

lib/spells/effects/RemoveObstacle.cpp

@@ -47,6 +47,7 @@ bool RemoveObstacle::applicable(Problem & problem, const Mechanics * m) const
 	{
 		return m->adaptProblem(ESpellCastProblem::NO_APPROPRIATE_TARGET, problem);
 	}
+	return true;
 }
 
 bool RemoveObstacle::applicable(Problem & problem, const Mechanics * m, const EffectTarget & target) const