Merge branch 'develop' of https://github.com/vcmi/vcmi into develop

.travis.linux

@@ -16,7 +16,7 @@ sudo apt-get update -qq
 
 sudo apt-get install -qq $SUPPORT
 sudo apt-get install -qq $PACKAGE
-sudo apt-get install -qq cmake libboost1.54-all-dev zlib1g-dev
+sudo apt-get install -qq cmake ninja-build libboost1.54-all-dev zlib1g-dev
 sudo apt-get install -qq libsdl2-dev libsdl2-image-dev libsdl2-mixer-dev libsdl2-ttf-dev
 sudo apt-get install -qq libavformat-dev libswscale-dev
 sudo apt-get install -qq qt57declarative

.travis.yml

@@ -14,7 +14,7 @@ before_install:
 before_script:
   - mkdir build
   - cd build
-  - cmake -G "Unix Makefiles" .. $VCMI_CMAKE_FLAGS
+  - if [[ $TRAVIS_BRANCH != 'coverity_scan' ]]; then cmake -G "Unix Makefiles" .. $VCMI_CMAKE_FLAGS; fi
 
 script:
   - test $TRAVIS_BRANCH != coverity_scan || exit 0
@@ -35,13 +35,13 @@ matrix:
   include:
     - os: linux
       compiler: clang
-      env: VCMI_PLATFORM='linux' REAL_CC=clang-3.4 REAL_CXX=clang++-3.4 PACKAGE=clang-3.4 SUPPORT=libstdc++-4.8-dev
+      env: VCMI_PLATFORM='linux' REAL_CC=clang-3.6 REAL_CXX=clang++-3.6 PACKAGE=clang-3.6 SUPPORT=libstdc++-4.8-dev
     - os: linux
       compiler: clang
       env: VCMI_PLATFORM='linux' REAL_CC=clang-3.5 REAL_CXX=clang++-3.5 PACKAGE=clang-3.5 SUPPORT=libstdc++-4.8-dev
     - os: linux
       compiler: clang
-      env: VCMI_PLATFORM='linux' REAL_CC=clang-3.6 REAL_CXX=clang++-3.6 PACKAGE=clang-3.6 SUPPORT=libstdc++-4.8-dev
+      env: VCMI_PLATFORM='linux' REAL_CC=clang-3.4 REAL_CXX=clang++-3.4 PACKAGE=clang-3.4 SUPPORT=libstdc++-4.8-dev
     - os: linux
       compiler: gcc
       env: VCMI_PLATFORM='linux' REAL_CC=gcc-4.8   REAL_CXX=g++-4.8     PACKAGE=g++-4.8   SUPPORT=
@@ -57,8 +57,8 @@ addons:
       name: vcmi/vcmi
       description: Build submitted via Travis CI
     notification_email: coverity@arseniyshestakov.com
-    build_command_prepend: cmake -G "Unix Makefiles" . -DENABLE_LAUNCHER=1 -DENABLE_TEST=1
+    build_command_prepend: "cov-configure --compiler clang-3.6 --comptype clangcc && cov-configure --comptype clangcxx --compiler clang++-3.6 && cmake -G Ninja .. -DCMAKE_BUILD_TYPE=DEBUG -DENABLE_LAUNCHER=1 -DENABLE_TEST=1"
-    build_command: make
+    build_command: ninja -j 2
     branch_pattern: coverity_scan
 
 notifications:

AI/BattleAI/BattleAI.cpp

@@ -13,30 +13,11 @@
 #include "EnemyInfo.h"
 #include "../../lib/spells/CSpellHandler.h"
 
-
-
-/*
-	//
-	// //set has its own order, so remove_if won't work. TODO - reuse for map
-	// template<typename Elem, typename Predicate>
-	// void erase_if(std::set<Elem> &setContainer, Predicate pred)
-	// {
-	// 	auto itr = setContainer.begin();
-	// 	auto endItr = setContainer.end();
-	// 	while(itr != endItr)
-	// 	{
-	// 		auto tmpItr = itr++;
-	// 		if(pred(*tmpItr))
-	// 			setContainer.erase(tmpItr);
-	// 	}
-	// }
-	*/
-
 #define LOGL(text) print(text)
 #define LOGFL(text, formattingEl) print(boost::str(boost::format(text) % formattingEl))
 
 CBattleAI::CBattleAI(void)
-	: side(-1)
+	: side(-1), wasWaitingForRealize(false), wasUnlockingGs(false)
 {
 }
 
@@ -82,8 +63,8 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 				return BattleAction::makeHeal(stack, woundHpToStack.rbegin()->second); //last element of the woundHpToStack is the most wounded stack
 		}
 
-		if(cb->battleCanCastSpell())
 		attemptCastingSpell();
+
 		if(auto ret = getCbc()->battleIsFinished())
 		{
 			//spellcast may finish battle
@@ -201,8 +182,14 @@ SpellTypes spellType(const CSpell *spell)
 
 void CBattleAI::attemptCastingSpell()
 {
+	auto hero = cb->battleGetMyHero();
+	if(!hero)
+		return;
+
+	if(!cb->battleCanCastSpell())
+		return;
+
 	LOGL("Casting spells sounds like fun. Let's see...");
-	auto hero = cb->battleGetMyHero(); //auto known = cb->battleGetFightingHero(side);
 	//Get all spells we can cast
 	std::vector<const CSpell*> possibleSpells;
 	vstd::copy_if(VLC->spellh->objects, std::back_inserter(possibleSpells), [this] (const CSpell *s) -> bool

AI/VCAI/AIUtility.cpp

@@ -297,6 +297,7 @@ ui64 evaluateDanger(const CGObjectInstance *obj)
 			return cre->getArmyStrength();
 		}
 	case Obj::CREATURE_GENERATOR1:
+	case Obj::CREATURE_GENERATOR4:
 		{
 			const CGDwelling *d = dynamic_cast<const CGDwelling*>(obj);
 			return d->getArmyStrength();

AI/VCAI/VCAI.h

@@ -83,6 +83,7 @@ struct SectorMap
 		Sector()
 		{
 			id = -1;
+			water = false;
 		}
 	};
 

client/CGameInfo.cpp

@@ -18,7 +18,9 @@ CClientState * CCS = nullptr;
 
 CGameInfo::CGameInfo()
 {
+	generaltexth = nullptr;
 	mh = nullptr;
+	townh = nullptr;
 }
 
 void CGameInfo::setFromLib()

client/CMT.cpp

@@ -619,17 +619,22 @@ void processCommand(const std::string &message)
 	}
 	else if(cn=="save")
 	{
+		if(!client)
+		{
+			std::cout << "Game in not active";
+			return;
+		}
 		std::string fname;
 		readed >> fname;
 		client->save(fname);
 	}
-	else if(cn=="load")
+//	else if(cn=="load")
-	{
+//	{
-		// TODO: this code should end the running game and manage to call startGame instead
+//		// TODO: this code should end the running game and manage to call startGame instead
-		std::string fname;
+//		std::string fname;
-		readed >> fname;
+//		readed >> fname;
-		client->loadGame(fname);
+//		client->loadGame(fname);
-	}
+//	}
 	else if(message=="get txt")
 	{
 		std::cout << "Command accepted.\t";
@@ -667,11 +672,6 @@ void processCommand(const std::string &message)
 	{
 		vm.insert(std::pair<std::string, po::variable_value>("onlyAI", po::variable_value()));
 	}
-	else if (cn == "ai")
-	{
-		VLC->IS_AI_ENABLED = !VLC->IS_AI_ENABLED;
-		std::cout << "Current AI status: " << (VLC->IS_AI_ENABLED ? "enabled" : "disabled") << std::endl;
-	}
 	else if(cn == "mp" && adventureInt)
 	{
 		if(const CGHeroInstance *h = dynamic_cast<const CGHeroInstance *>(adventureInt->selection))
@@ -837,6 +837,11 @@ void processCommand(const std::string &message)
 	else if(cn == "gosolo")
 	{
 		boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
+		if(!client)
+		{
+			std::cout << "Game in not active";
+			return;
+		}
 		PlayerColor color;
 		if(session["aiSolo"].Bool())
 		{
@@ -871,6 +876,11 @@ void processCommand(const std::string &message)
 		boost::to_lower(colorName);
 
 		boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
+		if(!client)
+		{
+			std::cout << "Game in not active";
+			return;
+		}
 		PlayerColor color;
 		if(LOCPLINT)
 			color = LOCPLINT->playerID;
@@ -1250,7 +1260,7 @@ void startGame(StartInfo * options, CConnection *serv/* = nullptr*/)
 		}
 	}
 
-    client = new CClient;
+    client = new CClient();
 	CPlayerInterface::howManyPeople = 0;
 	switch(options->mode) //new game
 	{

client/CPlayerInterface.cpp

@@ -87,7 +87,7 @@ CPlayerInterface * LOCPLINT;
 CBattleInterface * CPlayerInterface::battleInt;
 
 enum  EMoveState {STOP_MOVE, WAITING_MOVE, CONTINUE_MOVE, DURING_MOVE};
-CondSh<EMoveState> stillMoveHero; //used during hero movement
+CondSh<EMoveState> stillMoveHero(STOP_MOVE); //used during hero movement
 
 int CPlayerInterface::howManyPeople = 0;
 
@@ -1133,7 +1133,7 @@ void CPlayerInterface::showOkDialog(std::vector<Component> & components, const M
 	CInfoWindow::showOkDialog(str, &intComps, onOk, true, playerID);
 }
 
-void CPlayerInterface::showBlockingDialog( const std::string &text, const std::vector<Component> &components, QueryID askID, int soundID, bool selection, bool cancel )
+void CPlayerInterface::showBlockingDialog( const std::string &text, const std::vector<Component> &components, QueryID askID, const int soundID, bool selection, bool cancel )
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	waitWhileDialog();

client/CPlayerInterface.h

@@ -164,7 +164,7 @@ public:
 	void showInfoDialog(const std::string &text, const std::vector<Component*> &components, int soundID) override;
 	void showRecruitmentDialog(const CGDwelling *dwelling, const CArmedInstance *dst, int level) override;
 	void showShipyardDialog(const IShipyard *obj) override; //obj may be town or shipyard;
-	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, QueryID askID, int soundID, bool selection, bool cancel) override; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
+	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, QueryID askID, const int soundID, bool selection, bool cancel) override; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
 	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
 	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, QueryID queryID) override;
 	void showPuzzleMap() override;

client/CPreGame.cpp

@@ -620,6 +620,7 @@ CSelectionScreen::CSelectionScreen(CMenuScreen::EState Type, CMenuScreen::EMulti
 	if (screenType == CMenuScreen::campaignList)
 	{
 		opt = nullptr;
+		randMapTab = nullptr;
 	}
 	else
 	{
@@ -2250,7 +2251,7 @@ void InfoCard::changeSelection( const CMapInfo *to )
 void InfoCard::clickRight( tribool down, bool previousState )
 {
 	static const Rect flagArea(19, 397, 335, 23);
-	if(down && SEL->current && isItInLoc(flagArea, GH.current->motion.x, GH.current->motion.y))
+	if(SEL->current && down && SEL->current && isItInLoc(flagArea, GH.current->motion.x, GH.current->motion.y))
 		showTeamsPopup();
 }
 
@@ -3912,6 +3913,7 @@ CSavingScreen::~CSavingScreen()
 ISelectionScreenInfo::ISelectionScreenInfo(const std::map<ui8, std::string> *Names /*= nullptr*/)
 {
 	multiPlayer = CMenuScreen::SINGLE_PLAYER;
+	screenType = CMenuScreen::mainMenu;
 	assert(!SEL);
 	SEL = this;
 	current = nullptr;
@@ -4102,6 +4104,8 @@ void StartWithCurrentSettings::apply(CSelectionScreen *selScreen)
 
 CCampaignScreen::CCampaignButton::CCampaignButton(const JsonNode &config )
 {
+	OBJ_CONSTRUCTION_CAPTURING_ALL;
+
 	pos.x += config["x"].Float();
 	pos.y += config["y"].Float();
 	pos.w = 200;
@@ -4110,24 +4114,23 @@ CCampaignScreen::CCampaignButton::CCampaignButton(const JsonNode &config )
 	campFile = config["file"].String();
 	video = config["video"].String();
 
-	OBJ_CONSTRUCTION_CAPTURING_ALL;
-
 	status = config["open"].Bool() ? CCampaignScreen::ENABLED : CCampaignScreen::DISABLED;
 
 	CCampaignHeader header = CCampaignHandler::getHeader(campFile);
 	hoverText = header.name;
 
+	hoverLabel = nullptr;
 	if (status != CCampaignScreen::DISABLED)
 	{
 		addUsedEvents(LCLICK | HOVER);
-		image = new CPicture(config["image"].String());
+		new CPicture(config["image"].String());
 
 		hoverLabel = new CLabel(pos.w / 2, pos.h + 20, FONT_MEDIUM, CENTER, Colors::YELLOW, "");
 		parent->addChild(hoverLabel);
 	}
 
 	if (status == CCampaignScreen::COMPLETED)
-		checkMark = new CPicture("CAMPCHK");
+		new CPicture("CAMPCHK");
 }
 
 void CCampaignScreen::CCampaignButton::clickLeft(tribool down, bool previousState)
@@ -4141,12 +4144,15 @@ void CCampaignScreen::CCampaignButton::clickLeft(tribool down, bool previousStat
 }
 
 void CCampaignScreen::CCampaignButton::hover(bool on)
+{
+	if (hoverLabel)
 	{
 		if (on)
 			hoverLabel->setText(hoverText); // Shows the name of the campaign when you get into the bounds of the button
 		else
 			hoverLabel->setText(" ");
 	}
+}
 
 void CCampaignScreen::CCampaignButton::show(SDL_Surface * to)
 {
@@ -4198,7 +4204,7 @@ CCampaignScreen::CCampaignScreen(const JsonNode &config)
 
 	if (!config["exitbutton"].isNull())
 	{
-		back = createExitButton(config["exitbutton"]);
+		CButton * back = createExitButton(config["exitbutton"]);
 		back->hoverable = true;
 	}
 

client/CPreGame.h

@@ -129,7 +129,6 @@ class InfoCard : public CIntObject
 	std::shared_ptr<CAnimation> sFlags;
 public:
 	CPicture *bg;
-	CMenuScreen::EState type;
 
 	bool network;
 	bool chatOn;  //if chat is shown, then description is hidden
@@ -554,9 +553,6 @@ private:
 	class CCampaignButton : public CIntObject
 	{
 	private:
-		CPicture *image;
-		CPicture *checkMark;
-
 		CLabel *hoverLabel;
 		CampaignStatus status;
 
@@ -572,7 +568,6 @@ private:
 		void show(SDL_Surface * to) override;
 	};
 
-	CButton *back;
 	std::vector<CCampaignButton*> campButtons;
 	std::vector<CPicture*> images;
 

client/CVideoHandler.cpp

@@ -49,13 +49,20 @@ static si64 lodSeek(void * opaque, si64 pos, int whence)
 
 CVideoPlayer::CVideoPlayer()
 {
+	stream = -1;
 	format = nullptr;
-	frame = nullptr;
+	codecContext = nullptr;
 	codec = nullptr;
+	frame = nullptr;
 	sws = nullptr;
+	context = nullptr;
 	texture = nullptr;
 	dest = nullptr;
-	context = nullptr;
+	destRect = genRect(0,0,0,0);
+	pos = genRect(0,0,0,0);
+	refreshWait = 0;
+	refreshCount = 0;
+	doLoop = false;
 
 	// Register codecs. TODO: May be overkill. Should call a
 	// combination of av_register_input_format() /

client/battle/CBattleAnimations.cpp

@@ -134,10 +134,10 @@ bool CAttackAnimation::checkInitialConditions()
 
 CAttackAnimation::CAttackAnimation(CBattleInterface *_owner, const CStack *attacker, BattleHex _dest, const CStack *defender)
 	: CBattleStackAnimation(_owner, attacker),
+		shooting(false), group(CCreatureAnim::SHOOT_FRONT),
 		soundPlayed(false),
 		dest(_dest), attackedStack(defender), attackingStack(attacker)
 {
-
 	assert(attackingStack && "attackingStack is nullptr in CBattleAttack::CBattleAttack !\n");
 	bool isCatapultAttack = attackingStack->hasBonusOfType(Bonus::CATAPULT)
 							&& owner->getCurrentPlayerInterface()->cb->battleHexToWallPart(_dest) >= 0;
@@ -150,7 +150,7 @@ CAttackAnimation::CAttackAnimation(CBattleInterface *_owner, const CStack *attac
 CDefenceAnimation::CDefenceAnimation(StackAttackedInfo _attackedInfo, CBattleInterface * _owner)
 : CBattleStackAnimation(_owner, _attackedInfo.defender),
 attacker(_attackedInfo.attacker), rangedAttack(_attackedInfo.indirectAttack),
-killed(_attackedInfo.killed) 
+killed(_attackedInfo.killed), timeToWait(0)
 {
 	logAnim->debugStream() << "Created defence anim for " << _attackedInfo.defender->getName();
 }

client/battle/CBattleInterface.cpp

@@ -46,7 +46,7 @@
  *
  */
 
-CondSh<bool> CBattleInterface::animsAreDisplayed;
+CondSh<bool> CBattleInterface::animsAreDisplayed(false);
 
 static void onAnimationFinished(const CStack *stack, CCreatureAnimation *anim)
 {

client/battle/CBattleInterfaceClasses.cpp

@@ -191,7 +191,7 @@ void CBattleHero::clickLeft(tribool down, bool previousState)
 	if(myOwner->spellDestSelectMode) //we are casting a spell
 		return;
 
-	if(!down && myHero != nullptr && myOwner->myTurn && myOwner->getCurrentPlayerInterface()->cb->battleCanCastSpell()) //check conditions
+	if(myHero != nullptr && !down &&  myOwner->myTurn && myOwner->getCurrentPlayerInterface()->cb->battleCanCastSpell()) //check conditions
 	{
 		for(int it=0; it<GameConstants::BFIELD_SIZE; ++it) //do nothing when any hex is hovered - hero's animation overlaps battlefield
 		{

client/gui/CAnimation.cpp

@@ -1557,8 +1557,7 @@ bool CFadeAnimation::isFinished() const
 }
 
 CFadeAnimation::CFadeAnimation()
-	: fadingSurface(nullptr),
+	: delta(0),	fadingSurface(nullptr), fading(false), fadingCounter(0), shouldFreeSurface(false),
-	  fading(false),
 	  fadingMode(EMode::NONE)
 {
 }

client/gui/CGuiHandler.cpp

@@ -16,7 +16,7 @@
 extern std::queue<SDL_Event> events;
 extern boost::mutex eventsM;
 
-CondSh<bool> CGuiHandler::terminate_cond;
+CondSh<bool> CGuiHandler::terminate_cond(false);
 boost::thread_specific_ptr<bool> inGuiThread;
 
 SObjectConstruction::SObjectConstruction(CIntObject *obj)
@@ -430,7 +430,7 @@ void CGuiHandler::renderFrame()
 
 
 CGuiHandler::CGuiHandler()
-:lastClick(-500, -500)
+	: lastClick(-500, -500),lastClickTime(0), defActionsDef(0), captureChildren(false)
 {
 	curInt = nullptr;
 	current = nullptr;
@@ -546,6 +546,8 @@ CFramerateManager::CFramerateManager(int rate)
 	this->fps = 0;
 	this->accumulatedFrames = 0;
 	this->accumulatedTime = 0;
+	this->lastticks = 0;
+	this->timeElapsed = 0;
 }
 
 void CFramerateManager::init()

client/gui/CGuiHandler.h

@@ -77,6 +77,10 @@ public:
 	Point lastClick;
 	unsigned lastClickTime;
 
+	ui8 defActionsDef; //default auto actions
+	bool captureChildren; //all newly created objects will get their parents from stack and will be added to parents children list
+	std::list<CIntObject *> createdObj; //stack of objs being created
+
 	CGuiHandler();
 	~CGuiHandler();
 
@@ -99,9 +103,6 @@ public:
 	void fakeMouseMove();
 	void breakEventHandling(); //current event won't be propagated anymore
 	void drawFPSCounter(); // draws the FPS to the upper left corner of the screen
-	ui8 defActionsDef; //default auto actions
-	ui8 captureChildren; //all newly created objects will get their parents from stack and will be added to parents children list
-	std::list<CIntObject *> createdObj; //stack of objs being created
 
 	static SDL_Keycode arrowToNum(SDL_Keycode key); //converts arrow key to according numpad key
 	static SDL_Keycode numToDigit(SDL_Keycode key);//converts numpad digit key to normal digit key

client/mapHandler.cpp

@@ -709,6 +709,14 @@ CMapHandler::CMapPuzzleViewBlitter::CMapPuzzleViewBlitter(CMapHandler * parent)
 	unblittableObjects.push_back(Obj::HOLE);
 }
 
+CMapHandler::CMapBlitter::CMapBlitter(CMapHandler * p)
+	:parent(p), tileSize(0), halfTileSizeCeil(0), info(nullptr)
+{
+
+}
+
+CMapHandler::CMapBlitter::~CMapBlitter() = default;
+
 void CMapHandler::CMapBlitter::drawFrame(SDL_Surface * targetSurf) const
 {
 	Rect destRect(realTileRect);
@@ -1369,6 +1377,9 @@ CMapHandler::CMapHandler()
 	worldViewBlitter = new CMapWorldViewBlitter(this);
 	puzzleViewBlitter = new CMapPuzzleViewBlitter(this);
 	fadeAnimCounter = 0;
+	map = nullptr;
+	tilesW = tilesH = 0;
+	offsetX = offsetY = 0;
 
 	egdeAnimation = make_unique<CAnimation>("EDG");
 	egdeAnimation->preload();
@@ -1405,6 +1416,11 @@ void CMapHandler::discardWorldViewCache()
 	cache.discardWorldViewCache();
 }
 
+CMapHandler::CMapCache::CMapCache()
+{
+	worldViewCachedScale = 0;
+}
+
 void CMapHandler::CMapCache::discardWorldViewCache()
 {
 	for(auto & cache : data)

client/mapHandler.h

@@ -162,6 +162,7 @@ class CMapHandler
 		std::array< std::map<intptr_t, std::unique_ptr<IImage>>, (ui8)EMapCacheType::AFTER_LAST> data;
 		float worldViewCachedScale;
 	public:
+		CMapCache();
 		/// destroys all cached data (frees surfaces)
 		void discardWorldViewCache();
 		/// updates scale and determines if currently cached data is still valid
@@ -254,8 +255,8 @@ class CMapHandler
 		IImage * findFlagBitmapInternal(std::shared_ptr<CAnimation> animation, int anim, int group, ui8 dir, bool moving) const;
 
 	public:
-		CMapBlitter(CMapHandler * p) : parent(p) {}
+		CMapBlitter(CMapHandler * p);
-		virtual ~CMapBlitter(){}
+		virtual ~CMapBlitter();
 		void blit(SDL_Surface * targetSurf, const MapDrawingInfo * info);
 		/// helper method that chooses correct bitmap(s) for given object
 		AnimBitmapHolder findObjectBitmap(const CGObjectInstance * obj, int anim) const;

client/widgets/AdventureMapClasses.cpp

@@ -785,7 +785,12 @@ void CInfoBar::CVisibleInfo::loadGameStatus()
 	//get amount of halls of each level
 	std::vector<int> halls(4, 0);
 	for(auto town : LOCPLINT->towns)
-		halls[town->hallLevel()]++;
+	{
+		int hallLevel = town->hallLevel();
+		//negative value means no village hall, unlikely but possible
+		if(hallLevel >= 0)
+			halls.at(hallLevel)++;
+	}
 
 	std::vector<PlayerColor> allies, enemies;
 

client/widgets/Buttons.cpp

@@ -24,39 +24,6 @@
  *
  */
 
-ClickableArea::ClickableArea(CIntObject * object, CFunctionList<void()> callback):
-	callback(callback),
-	area(nullptr)
-{
-	if (object)
-		pos = object->pos;
-	setArea(object);
-}
-
-void ClickableArea::addCallback(std::function<void()> callback)
-{
-	this->callback += callback;
-}
-
-void ClickableArea::setArea(CIntObject * object)
-{
-	delete area;
-	area = object;
-	addChild(area);
-	pos.w = object->pos.w;
-	pos.h = object->pos.h;
-}
-
-void ClickableArea::onClick()
-{
-	callback();
-}
-
-void ClickableArea::clickLeft(tribool down, bool previousState)
-{
-	if (down)
-		onClick();
-}
 
 void CButton::update()
 {

client/widgets/Buttons.h

@@ -26,24 +26,6 @@ namespace config
  *
  */
 
-class ClickableArea : public CIntObject //TODO: derive from LRCLickableArea? Or somehow use its right-click/hover data?
-{
-	CFunctionList<void()> callback;
-
-	CIntObject * area;
-
-protected:
-	void onClick();
-
-public:
-	ClickableArea(CIntObject * object, CFunctionList<void()> callback);
-
-	void addCallback(std::function<void()> callback);
-	void setArea(CIntObject * object);
-
-	void clickLeft(tribool down, bool previousState) override;
-};
-
 /// Typical Heroes 3 button which can be inactive or active and can
 /// hold further information if you right-click it
 class CButton : public CKeyShortcut
@@ -151,13 +133,6 @@ public:
 	void addCallback(std::function<void(bool)> callback);
 };
 
-class ClickableToggle : public ClickableArea, public CToggleBase
-{
-public:
-	ClickableToggle(CIntObject * object, CFunctionList<void()> selectFun, CFunctionList<void()> deselectFun);
-	void clickLeft(tribool down, bool previousState) override;
-};
-
 /// A button which can be selected/deselected, checkbox
 class CToggleButton : public CButton, public CToggleBase
 {
@@ -224,9 +199,6 @@ class CSlider : public CIntObject
 	int capacity;//how many elements can be active at same time (e.g. hero list = 5)
 	int positions; //number of highest position (0 if there is only one)
 	bool horizontal;
-	bool wheelScrolling;
-	bool keyScrolling;
-
 	int amount; //total amount of elements (e.g. hero list = 0-8)
 	int value; //first active element
 	int scrollStep; // how many elements will be scrolled via one click, default = 1

client/widgets/CArtifactHolder.cpp

@@ -239,7 +239,7 @@ bool CHeroArtPlace::askToAssemble(const CArtifactInstance *art, ArtifactPosition
 
 void CHeroArtPlace::clickRight(tribool down, bool previousState)
 {
-	if(down && ourArt && !locked && text.size() && !picked)  //if there is no description or it's a lock, do nothing ;]
+	if(ourArt && down && ourArt && !locked && text.size() && !picked)  //if there is no description or it's a lock, do nothing ;]
 	{
 		if (slotID < GameConstants::BACKPACK_START)
 		{
@@ -946,6 +946,7 @@ bool CArtifactsOfHero::SCommonPart::Artpos::valid()
 
 CArtPlace::CArtPlace(Point position, const CArtifactInstance * Art) : ourArt(Art)
 {
+	image = nullptr;
 	pos += position;
 	pos.w = pos.h = 44;
 }
@@ -968,13 +969,13 @@ CCommanderArtPlace::CCommanderArtPlace(Point position, const CGHeroInstance * co
 
 void CCommanderArtPlace::clickLeft(tribool down, bool previousState)
 {
-	if (down && ourArt && text.size())
+	if (ourArt && text.size() && down)
 		LOCPLINT->showYesNoDialog(CGI->generaltexth->localizedTexts["commanderWindow"]["artifactMessage"].String(), [this] { returnArtToHeroCallback(); }, [] {});
 }
 
 void CCommanderArtPlace::clickRight(tribool down, bool previousState)
 {
-	if (down && ourArt && text.size())
+	if (ourArt && text.size() && down)
 		CArtPlace::clickRight(down, previousState);
 }
 

client/widgets/CGarrisonInt.cpp

@@ -269,7 +269,7 @@ bool CGarrisonSlot::mustForceReselection() const
 
 void CGarrisonSlot::clickRight(tribool down, bool previousState)
 {
-	if(down && creature)
+	if(creature && down)
 	{
 		GH.pushInt(new CStackWindow(myStack, true));
 	}
@@ -475,6 +475,7 @@ CGarrisonInt::CGarrisonInt(int x, int y, int inx, const Point &garsOffset,
     inSplittingMode(false),
     interx(inx),
     garOffset(garsOffset),
+    pb(false),
     smallIcons(smallImgs),
     removableUnits(_removableUnits),
     twoRows(_twoRows)
@@ -547,17 +548,23 @@ CGarrisonWindow::CGarrisonWindow( const CArmedInstance *up, const CGHeroInstance
 	quit = new CButton(Point(399, 314), "IOK6432.DEF", CButton::tooltip(CGI->generaltexth->tcommands[8], ""), [&]{ close(); }, SDLK_RETURN);
 
 	std::string titleText;
-	if (garr->armedObjs[1]->tempOwner == garr->armedObjs[0]->tempOwner)
+	if (down->tempOwner == up->tempOwner)
 		titleText = CGI->generaltexth->allTexts[709];
 	else
+	{
+		//assume that this is joining monsters dialog
+		if(up->Slots().size() > 0)
 		{
 			titleText = CGI->generaltexth->allTexts[35];
-		boost::algorithm::replace_first(titleText, "%s", garr->armedObjs[0]->Slots().begin()->second->type->namePl);
+			boost::algorithm::replace_first(titleText, "%s", up->Slots().begin()->second->type->namePl);
+		}
+		else
+			logGlobal->error("Invalid armed instance for garrison window.");
 	}
 	new CLabel(275, 30, FONT_BIG, CENTER, Colors::YELLOW, titleText);
 
-	new CAnimImage("CREST58", garr->armedObjs[0]->getOwner().getNum(), 0, 28, 124);
+	new CAnimImage("CREST58", up->getOwner().getNum(), 0, 28, 124);
-	new CAnimImage("PortraitsLarge", dynamic_cast<const CGHeroInstance*>(garr->armedObjs[1])->portrait, 0, 29, 222);
+	new CAnimImage("PortraitsLarge", down->portrait, 0, 29, 222);
 }
 
 CGarrisonHolder::CGarrisonHolder()

client/widgets/CGarrisonInt.h

@@ -78,19 +78,14 @@ public:
 	std::vector<CButton *> splitButtons;  ///< May be empty if no buttons
 
 	SlotID p2; ///< TODO: comment me
-	int	shiftPos; ///< 1st slot of the second row, set shiftPoint for effect
 	bool pb,
 		 smallIcons,      ///< true - 32x32 imgs, false - 58x64
 		 removableUnits,  ///< player Can remove units from up
 		 twoRows,         ///< slots Will be placed in 2 rows
 		 owned[2];        ///< player Owns up or down army ([0] upper, [1] lower)
 
-// 	const CCreatureSet *set1;  ///< Top set of creatures
-// 	const CCreatureSet *set2;  ///< Bottom set of creatures
-
 	std::vector<CGarrisonSlot*> slotsUp, slotsDown;  ///< Slots of upper and lower garrison
 	const CArmedInstance *armedObjs[2];  ///< [0] is upper, [1] is down
-	//const CArmedInstance *oup, *odown;  ///< Upper and lower garrisons (heroes or towns)
 
 	void setArmy(const CArmedInstance *army, bool bottomGarrison);
 	void addSplitBtn(CButton * button);

client/widgets/MiscWidgets.cpp

@@ -94,7 +94,7 @@ void LRClickableAreaWTextComp::clickLeft(tribool down, bool previousState)
 }
 
 LRClickableAreaWTextComp::LRClickableAreaWTextComp(const Rect &Pos, int BaseType)
-	: LRClickableAreaWText(Pos), baseType(BaseType), bonusValue(-1)
+	: LRClickableAreaWText(Pos), baseType(BaseType), bonusValue(-1), type(-1)
 {
 }
 
@@ -134,13 +134,13 @@ CHeroArea::CHeroArea(int x, int y, const CGHeroInstance * _hero):hero(_hero)
 
 void CHeroArea::clickLeft(tribool down, bool previousState)
 {
-	if((!down) && previousState && hero)
+	if(hero && (!down) && previousState)
 		LOCPLINT->openHeroWindow(hero);
 }
 
 void CHeroArea::clickRight(tribool down, bool previousState)
 {
-	if((!down) && previousState && hero)
+	if(hero && (!down) && previousState)
 		LOCPLINT->openHeroWindow(hero);
 }
 
@@ -154,7 +154,7 @@ void CHeroArea::hover(bool on)
 
 void LRClickableAreaOpenTown::clickLeft(tribool down, bool previousState)
 {
-	if((!down) && previousState && town)
+	if(town && (!down) && previousState)
 	{
 		LOCPLINT->openTownWindow(town);
 		if ( type == 2 )
@@ -166,12 +166,12 @@ void LRClickableAreaOpenTown::clickLeft(tribool down, bool previousState)
 
 void LRClickableAreaOpenTown::clickRight(tribool down, bool previousState)
 {
-	if((!down) && previousState && town)
+	if(town && (!down) && previousState)
 		LOCPLINT->openTownWindow(town);//TODO: popup?
 }
 
-LRClickableAreaOpenTown::LRClickableAreaOpenTown()
+LRClickableAreaOpenTown::LRClickableAreaOpenTown(const Rect & Pos, const CGTownInstance * Town)
-	: LRClickableAreaWTextComp(Rect(0,0,0,0), -1)
+	: LRClickableAreaWTextComp(Pos, -1), town(Town)
 {
 }
 

client/widgets/MiscWidgets.h

@@ -139,7 +139,7 @@ public:
 	const CGTownInstance * town;
 	void clickLeft(tribool down, bool previousState) override;
 	void clickRight(tribool down, bool previousState) override;
-	LRClickableAreaOpenTown();
+	LRClickableAreaOpenTown(const Rect & Pos, const CGTownInstance * Town);
 };
 
 class MoraleLuckBox : public LRClickableAreaWTextComp

client/widgets/TextControls.cpp

@@ -577,6 +577,7 @@ void CTextInput::numberFilter(std::string & text, const std::string & oldText, i
 
 CFocusable::CFocusable()
 {
+	focus = false;
 	focusables.push_back(this);
 }
 

client/windows/CAdvmapInterface.cpp

@@ -86,6 +86,7 @@ static void setScrollingCursor(ui8 direction)
 
 CTerrainRect::CTerrainRect()
 	: fadeSurface(nullptr),
+	  lastRedrawStatus(EMapAnimRedrawStatus::OK),
 	  fadeAnim(new CFadeAnimation()),
 	  curHoveredTile(-1,-1,-1),
 	  currentPath(nullptr)

client/windows/CAdvmapInterface.h

@@ -43,7 +43,7 @@ enum class EAdvMapMode
 class CAdventureOptions : public CWindowObject
 {
 public:
-	CButton *exit, *viewWorld, *puzzle, *dig, *scenInfo, *replay;
+	CButton *exit, *viewWorld, *puzzle, *dig, *scenInfo/*, *replay*/;
 
 	CAdventureOptions();
 	static void showScenarioInfo();

client/windows/CCastleInterface.cpp

@@ -109,7 +109,7 @@ void CBuildingRect::hover(bool on)
 
 void CBuildingRect::clickLeft(tribool down, bool previousState)
 {
-	if( previousState && !down && area && (parent->selectedBuilding==this) && getBuilding() )
+	if( previousState && getBuilding() && area && !down && (parent->selectedBuilding==this))
 		if (!CSDL_Ext::isTransparent(area, GH.current->motion.x-pos.x, GH.current->motion.y-pos.y) ) //inside building image
 			parent->buildingClicked(getBuilding()->bid);
 }
@@ -398,7 +398,7 @@ void CHeroGSlot::clickLeft(tribool down, bool previousState)
 
 void CHeroGSlot::clickRight(tribool down, bool previousState)
 {
-	if(down && hero)
+	if(hero && down)
 	{
 		GH.pushInt(new CInfoBoxPopup(Point(pos.x + 175, pos.y + 100), hero));
 	}
@@ -1014,6 +1014,7 @@ CCreaInfo::CCreaInfo(Point position, const CGTownInstance *Town, int Level, bool
 		level = -1;
 		label = nullptr;
 		picture = nullptr;
+		creature = nullptr;
 		return;//No creature
 	}
 	addUsedEvents(LCLICK | RCLICK | HOVER);
@@ -1125,6 +1126,8 @@ CTownInfo::CTownInfo(int posX, int posY, const CGTownInstance* Town, bool townHa
 	pos.y += posY;
 	int buildID;
 
+	picture = nullptr;
+
 	if (townHall)
 	{
 		buildID = 10 + town->hallLevel();
@@ -1134,7 +1137,7 @@ CTownInfo::CTownInfo(int posX, int posY, const CGTownInstance* Town, bool townHa
 	{
 		buildID = 6 + town->fortLevel();
 		if (buildID == 6)
-			return;
+			return;//FIXME: suspicious statement, fix or comment
 		picture = new CAnimImage("ITMCL.DEF", town->fortLevel()-1);
 	}
 	building = town->town->buildings.at(BuildingID(buildID));
@@ -1154,7 +1157,7 @@ void CTownInfo::hover(bool on)
 
 void CTownInfo::clickRight(tribool down, bool previousState)
 {
-	if(down && building)
+	if(building && down)
 		CRClickPopup::createAndPush(CInfoWindow::genText(building->Name(), building->Description()),
 		                            new CComponent(CComponent::building, building->town->faction->index, building->bid));
 
@@ -1302,11 +1305,11 @@ CHallInterface::CBuildingBox::CBuildingBox(int x, int y, const CGTownInstance *
 	static int panelIndex[12] = { 3,  3,  3, 0, 0, 2, 2,  1, 2, 2,  3,  3};
 	static int  iconIndex[12] = {-1, -1, -1, 0, 0, 1, 2, -1, 1, 1, -1, -1};
 
-	picture = new CAnimImage(town->town->clientInfo.buildingsIcons, building->bid, 0, 2, 2);
+	new CAnimImage(town->town->clientInfo.buildingsIcons, building->bid, 0, 2, 2);
-	panel = new CAnimImage("TPTHBAR", panelIndex[state], 0,   1, 73);
+	new CAnimImage("TPTHBAR", panelIndex[state], 0,   1, 73);
 	if (iconIndex[state] >=0)
-		icon  = new CAnimImage("TPTHCHK",  iconIndex[state], 0, 136, 56);
+		new CAnimImage("TPTHCHK",  iconIndex[state], 0, 136, 56);
-	label = new CLabel(75, 81, FONT_SMALL, CENTER, Colors::WHITE, building->Name());
+	new CLabel(75, 81, FONT_SMALL, CENTER, Colors::WHITE, building->Name());
 
 	//todo: add support for all possible states
 	if(state >= EBuildingState::BUILDING_ERROR)
@@ -1427,12 +1430,12 @@ CBuildWindow::CBuildWindow(const CGTownInstance *Town, const CBuilding * Buildin
 		std::string tooltipYes = boost::str(boost::format(CGI->generaltexth->allTexts[595]) % building->Name());
 		std::string tooltipNo  = boost::str(boost::format(CGI->generaltexth->allTexts[596]) % building->Name());
 
-		buy = new CButton(Point(45, 446), "IBUY30", CButton::tooltip(tooltipYes), [&]{ buyFunc(); }, SDLK_RETURN);
+		CButton * buy = new CButton(Point(45, 446), "IBUY30", CButton::tooltip(tooltipYes), [&]{ buyFunc(); }, SDLK_RETURN);
 		buy->borderColor = Colors::METALLIC_GOLD;
-
-		cancel = new CButton(Point(290, 445), "ICANCEL", CButton::tooltip(tooltipNo), [&] { close();}, SDLK_ESCAPE);
-		cancel->borderColor = Colors::METALLIC_GOLD;
 		buy->block(state!=7 || LOCPLINT->playerID != town->tempOwner);
+
+		CButton * cancel = new CButton(Point(290, 445), "ICANCEL", CButton::tooltip(tooltipNo), [&] { close();}, SDLK_ESCAPE);
+		cancel->borderColor = Colors::METALLIC_GOLD;
 	}
 }
 
@@ -1597,10 +1600,11 @@ CFortScreen::RecruitArea::RecruitArea(int posX, int posY, const CGTownInstance *
 		addUsedEvents(LCLICK | RCLICK | HOVER);//Activate only if dwelling is present
 
 	icons = new CPicture("TPCAINFO", 261, 3);
+
 	if (getMyBuilding() != nullptr)
 	{
-		buildingPic = new CAnimImage(town->town->clientInfo.buildingsIcons, getMyBuilding()->bid, 0, 4, 21);
+		new CAnimImage(town->town->clientInfo.buildingsIcons, getMyBuilding()->bid, 0, 4, 21);
-		dwellingName = new CLabel(78, 101, FONT_SMALL, CENTER, Colors::WHITE, getMyBuilding()->Name());
+		new CLabel(78, 101, FONT_SMALL, CENTER, Colors::WHITE, getMyBuilding()->Name());
 
 		if (vstd::contains(town->builtBuildings, getMyBuilding()->bid))
 		{
@@ -1613,8 +1617,8 @@ CFortScreen::RecruitArea::RecruitArea(int posX, int posY, const CGTownInstance *
 	if (getMyCreature() != nullptr)
 	{
 		hoverText = boost::str(boost::format(CGI->generaltexth->tcommands[21]) % getMyCreature()->namePl);
-		creatureAnim = new CCreaturePic(159, 4, getMyCreature(), false);
+		new CCreaturePic(159, 4, getMyCreature(), false);
-		creatureName = new CLabel(78,  11, FONT_SMALL, CENTER, Colors::WHITE, getMyCreature()->namePl);
+		new CLabel(78,  11, FONT_SMALL, CENTER, Colors::WHITE, getMyCreature()->namePl);
 
 		Rect sizes(287, 4, 96, 18);
 		values.push_back(new LabeledValue(sizes, CGI->generaltexth->allTexts[190], CGI->generaltexth->fcommands[0], getMyCreature()->Attack()));
@@ -1660,9 +1664,6 @@ void CFortScreen::RecruitArea::clickRight(tribool down, bool previousState)
 	clickLeft(down, false); //r-click does same as l-click - opens recr. window
 }
 
-
-
-
 CMageGuildScreen::CMageGuildScreen(CCastleInterface * owner,std::string imagem) :CWindowObject(BORDERED,imagem)
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL;

client/windows/CCastleInterface.h

@@ -240,12 +240,6 @@ class CHallInterface : public CWindowObject
 		const CBuilding * building;
 
 		ui32 state;//Buildings::EBuildStructure enum
-
-		CAnimImage * picture;
-		CAnimImage * panel;
-		CAnimImage * icon;
-		CLabel * label;
-
 	public:
 		CBuildingBox(int x, int y, const CGTownInstance * Town, const CBuilding * Building);
 		void hover(bool on) override;
@@ -270,12 +264,8 @@ class CBuildWindow: public CWindowObject
 	const CGTownInstance *town;
 	const CBuilding *building;
 
-	CButton *buy;
-	CButton *cancel;
-
 	std::string getTextForState(int state);
 	void buyFunc();
-
 public:
 	CBuildWindow(const CGTownInstance *Town, const CBuilding * building, int State, bool rightClick); //c-tor
 };
@@ -303,14 +293,10 @@ class CFortScreen : public CWindowObject
 		int level;
 
 		std::string hoverText;
-		CLabel * creatureName;
-		CLabel * dwellingName;
 		CLabel * availableCount;
 
 		std::vector<LabeledValue*> values;
 		CPicture *icons;
-		CAnimImage * buildingPic;
-		CCreaturePic *creatureAnim;
 
 		const CCreature * getMyCreature();
 		const CBuilding * getMyBuilding();

client/windows/CCreatureWindow.cpp

@@ -103,7 +103,7 @@ StackWindowInfo::StackWindowInfo():
 
 void CStackWindow::CWindowSection::createBackground(std::string path)
 {
-	background = new CPicture("stackWindow/" + path);
+	CPicture * background = new CPicture("stackWindow/" + path);
 	pos = background->pos;
 }
 

client/windows/CCreatureWindow.h

@@ -49,8 +49,6 @@ class CStackWindow : public CWindowObject
 	{
 		CStackWindow *parent;
 
-		CPicture *background;
-
 		void createBackground(std::string path);
 		void createBonusItem(size_t index, Point position);
 

client/windows/CKingdomInterface.cpp

@@ -792,9 +792,7 @@ CTownItem::CTownItem(const CGTownInstance* Town):
 	size_t iconIndex = town->town->clientInfo.icons[town->hasFort()][town->builded >= CGI->modh->settings.MAX_BUILDING_PER_TURN];
 
 	picture = new CAnimImage("ITPT", iconIndex, 0, 5, 6);
-	townArea = new LRClickableAreaOpenTown;
+	new LRClickableAreaOpenTown(Rect(5, 6, 58, 64), town);
-	townArea->pos = Rect(pos.x+5, pos.y+6, 58, 64);
-	townArea->town = town;
 
 	for (size_t i=0; i<town->creatures.size(); i++)
 	{

client/windows/CKingdomInterface.h

@@ -265,7 +265,6 @@ class CTownItem : public CIntObject, public CGarrisonHolder
 	CLabel *name;
 	CLabel *income;
 	CGarrisonInt *garr;
-	LRClickableAreaOpenTown *townArea;
 
 	HeroSlots *heroes;
 	CTownInfo *hall, *fort;

client/windows/CSpellWindow.cpp

@@ -529,7 +529,7 @@ CSpellWindow::SpellArea::~SpellArea()
 
 void CSpellWindow::SpellArea::clickLeft(tribool down, bool previousState)
 {
-	if(!down && mySpell)
+	if(mySpell && !down)
 	{
 		int spellCost = owner->myInt->cb->getSpellCost(mySpell, owner->myHero);
 		if(spellCost > owner->myHero->mana) //insufficient mana
@@ -752,7 +752,7 @@ void CSpellWindow::SpellArea::clickLeft(tribool down, bool previousState)
 
 void CSpellWindow::SpellArea::clickRight(tribool down, bool previousState)
 {
-	if(down && mySpell)
+	if(mySpell && down)
 	{
 		std::string dmgInfo;
 		int causedDmg = owner->myInt->cb->estimateSpellDamage(mySpell, owner->myHero);

client/windows/GUIClasses.cpp

@@ -788,13 +788,13 @@ void CTavernWindow::show(SDL_Surface * to)
 
 void CTavernWindow::HeroPortrait::clickLeft(tribool down, bool previousState)
 {
-	if(previousState && !down && h)
+	if(h && previousState && !down)
 		*_sel = _id;
 }
 
 void CTavernWindow::HeroPortrait::clickRight(tribool down, bool previousState)
 {
-	if(down && h)
+	if(h && down)
 	{
 		GH.pushInt(new CRClickPopupInt(new CHeroWindow(h), true));
 	}
@@ -1771,11 +1771,10 @@ void CObjectListWindow::init(CIntObject * titlePic, std::string _title, std::str
 
 	if (titlePic)
 	{
-		titleImage = titlePic;
+		addChild(titlePic);
-		addChild(titleImage);
+		titlePic->recActions = defActions;
-		titleImage->recActions = defActions;
+		titlePic->pos.x = pos.w/2 + pos.x - titlePic->pos.w/2;
-		titleImage->pos.x = pos.w/2 + pos.x - titleImage->pos.w/2;
+		titlePic->pos.y =75 + pos.y - titlePic->pos.h/2;
-		titleImage->pos.y =75 + pos.y - titleImage->pos.h/2;
 	}
 	list = new CListBox(std::bind(&CObjectListWindow::genItem, this, _1), CListBox::DestroyFunc(),
 		Point(14, 151), Point(0, 25), 9, items.size(), 0, 1, Rect(262, -32, 256, 256) );

client/windows/GUIClasses.h

@@ -161,7 +161,6 @@ class CObjectListWindow : public CWindowObject
 	CLabel * descr;
 
 	CListBox * list;
-	CIntObject * titleImage;//title image (castle gate\town portal picture)
 	CButton *ok, *exit;
 
 	std::vector< std::pair<int, std::string> > items;//all items present in list
@@ -243,7 +242,6 @@ public:
 
 	} *h1, *h2; //recruitable heroes
 
-	CGStatusBar *bar; //tavern's internal status bar
 	int selected;//0 (left) or 1 (right)
 	int oldSelected;//0 (left) or 1 (right)
 

client/windows/InfoWindows.h

@@ -47,7 +47,6 @@ public:
 	CTextBox *text;
 	std::vector<CButton *> buttons;
 	std::vector<CComponent*> components;
-	CSlider *slider;
 
 	void setDelComps(bool DelComps);
 	virtual void close();

lib/CArtHandler.cpp

@@ -76,6 +76,9 @@ CArtifact::CArtifact()
 	possibleSlots[ArtBearer::HERO]; //we want to generate map entry even if it will be empty
 	possibleSlots[ArtBearer::CREATURE]; //we want to generate map entry even if it will be empty
 	possibleSlots[ArtBearer::COMMANDER];
+	iconIndex = ArtifactID::NONE;
+	price = 0;
+	aClass = ART_SPECIAL;
 }
 
 CArtifact::~CArtifact()
@@ -1337,17 +1340,6 @@ bool CArtifactSet::isPositionFree(ArtifactPosition pos, bool onlyLockCheck /*= f
 	return true; //no slot means not used
 }
 
-si32 CArtifactSet::getArtTypeId(ArtifactPosition pos) const
-{
-	const CArtifactInstance * const a = getArt(pos);
-	if(!a)
-	{
-		logGlobal->warnStream() << (dynamic_cast<const CGHeroInstance*>(this))->name << " has no artifact at " << pos << " (getArtTypeId)";
-		return -1;
-	}
-	return a->artType->id;
-}
-
 ArtSlotInfo & CArtifactSet::retreiveNewArtSlot(ArtifactPosition slot)
 {
 	assert(!vstd::contains(artifactsWorn, slot));

lib/CArtHandler.h

@@ -54,6 +54,12 @@ public:
 	std::string large; // big image for cutom artifacts, used in drag & drop
 	std::string advMapDef; //used for adventure map object
 	si32 iconIndex;
+	ui32 price;
+	std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition> > possibleSlots; //Bearer Type => ids of slots where artifact can be placed
+	std::unique_ptr<std::vector<CArtifact *> > constituents; // Artifacts IDs a combined artifact consists of, or nullptr.
+	std::vector<CArtifact *> constituentOf; // Reverse map of constituents - combined arts that include this art
+	EartClass aClass;
+	ArtifactID id;
 
 	const std::string &Name() const; //getter
 	const std::string &Description() const; //getter
@@ -68,13 +74,6 @@ public:
 
 	virtual void levelUpArtifact (CArtifactInstance * art){};
 
-	ui32 price;
-	std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition> > possibleSlots; //Bearer Type => ids of slots where artifact can be placed
-	std::unique_ptr<std::vector<CArtifact *> > constituents; // Artifacts IDs a combined artifact consists of, or nullptr.
-	std::vector<CArtifact *> constituentOf; // Reverse map of constituents - combined arts that include this art
-	EartClass aClass;
-	ArtifactID id;
-
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CBonusSystemNode&>(*this);
@@ -321,7 +320,6 @@ public:
 	/// Checks if hero possess artifact of given id (either in backack or worn)
 	bool hasArt(ui32 aid, bool onlyWorn = false, bool searchBackpackAssemblies = false) const;
 	bool isPositionFree(ArtifactPosition pos, bool onlyLockCheck = false) const;
-	si32 getArtTypeId(ArtifactPosition pos) const;
 
 	virtual ArtBearer::ArtBearer bearerType() const = 0;
 	virtual ~CArtifactSet() = default;

lib/CBattleCallback.cpp

@@ -356,7 +356,9 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(con
 		return ESpellCastProblem::INVALID;
 	}
 	const PlayerColor player = caster->getOwner();
-	const ui8 side = playerToSide(player);
+	const si8 side = playerToSide(player);
+	if(side < 0)
+		return ESpellCastProblem::INVALID;
 	if(!battleDoWeKnowAbout(side))
 	{
 		logGlobal->warnStream() << "You can't check if enemy can cast given spell!";
@@ -391,7 +393,7 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(con
 bool CBattleInfoEssentials::battleCanFlee(PlayerColor player) const
 {
 	RETURN_IF_NOT_BATTLE(false);
-	ui8 mySide = playerToSide(player);
+	const si8 mySide = playerToSide(player);
 	const CGHeroInstance *myHero = battleGetFightingHero(mySide);
 
 	//current player have no hero
@@ -413,7 +415,7 @@ bool CBattleInfoEssentials::battleCanFlee(PlayerColor player) const
 	return true;
 }
 
-ui8 CBattleInfoEssentials::playerToSide(PlayerColor player) const
+si8 CBattleInfoEssentials::playerToSide(PlayerColor player) const
 {
 	RETURN_IF_NOT_BATTLE(-1);
 	int ret = vstd::find_pos_if(getBattle()->sides, [=](const SideInBattle &side){ return side.color == player; });
@@ -426,8 +428,8 @@ ui8 CBattleInfoEssentials::playerToSide(PlayerColor player) const
 bool CBattleInfoEssentials::playerHasAccessToHeroInfo(PlayerColor player, const CGHeroInstance * h) const
 {
 	RETURN_IF_NOT_BATTLE(false);
-	ui8 playerSide = playerToSide(player);
+	const si8 playerSide = playerToSide(player);
-	if (playerSide != (ui8)-1)
+	if (playerSide > 0)
 	{
 		if (getBattle()->sides[!playerSide].hero == h)
 			return true;
@@ -1714,7 +1716,11 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastThisSpell
 		return ESpellCastProblem::INVALID;
 	}
 	const PlayerColor player = caster->getOwner();
-	const ui8 side = playerToSide(player);
+	const si8 side = playerToSide(player);
+
+	if(side < 0)
+		return ESpellCastProblem::INVALID;
+
 	if(!battleDoWeKnowAbout(side))
 		return ESpellCastProblem::INVALID;
 
@@ -1727,7 +1733,12 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastThisSpell
 	case ECastingMode::HERO_CASTING:
 		{
 			const CGHeroInstance * castingHero = dynamic_cast<const CGHeroInstance *>(caster);//todo: unify hero|creature spell cost
-			assert(castingHero);
+			if(!castingHero)
+			{
+				logGlobal->error("battleCanCastThisSpell: invalid caster");
+				return ESpellCastProblem::INVALID;
+			}
+
 			if(!castingHero->getArt(ArtifactPosition::SPELLBOOK))
 				return ESpellCastProblem::NO_SPELLBOOK;
 			if(!castingHero->canCastThisSpell(spell))
@@ -1994,13 +2005,17 @@ int CBattleInfoCallback::battleGetSurrenderCost(PlayerColor Player) const
 	if(!battleCanSurrender(Player))
 		return -1;
 
+	const si8 playerSide = playerToSide(Player);
+	if(playerSide < 0)
+		return -1;
+
 	int ret = 0;
 	double discount = 0;
-	for(const CStack *s : battleAliveStacks(playerToSide(Player)))
+	for(const CStack *s : battleAliveStacks(playerSide))
 		if(s->base) //we pay for our stack that comes from our army slots - condition eliminates summoned cres and war machines
 			ret += s->getCreature()->cost[Res::GOLD] * s->count;
 
-	if(const CGHeroInstance *h = battleGetFightingHero(playerToSide(Player)))
+	if(const CGHeroInstance *h = battleGetFightingHero(playerSide))
 		discount += h->valOfBonuses(Bonus::SURRENDER_DISCOUNT);
 
 	ret *= (100.0 - discount) / 100.0;

lib/CBattleCallback.h

@@ -189,7 +189,7 @@ public:
 	si8 battleGetTacticsSide() const; //returns which side is in tactics phase, undefined if none (?)
 	bool battleCanFlee(PlayerColor player) const;
 	bool battleCanSurrender(PlayerColor player) const;
-	ui8 playerToSide(PlayerColor player) const;
+	si8 playerToSide(PlayerColor player) const;
 	bool playerHasAccessToHeroInfo(PlayerColor player, const CGHeroInstance * h) const;
 	ui8 battleGetSiegeLevel() const; //returns 0 when there is no siege, 1 if fort, 2 is citadel, 3 is castle
 	bool battleHasHero(ui8 side) const;

lib/CCreatureHandler.cpp

@@ -105,9 +105,15 @@ si32 CCreature::maxAmount(const std::vector<si32> &res) const //how many creatur
 
 CCreature::CCreature()
 {
-	doubleWide = false;
 	setNodeType(CBonusSystemNode::CREATURE);
+	faction = 0;
+	level = 0;
+	fightValue = AIValue = growth = hordeGrowth = ammMin = ammMax = 0;
+	doubleWide = false;
+	special = true;
+	iconIndex = -1;
 }
+
 void CCreature::addBonus(int val, Bonus::BonusType type, int subtype /*= -1*/)
 {
 	auto added = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::CREATURE_ABILITY, val, idNumber, subtype, Bonus::BASE_NUMBER);

lib/CGameState.cpp

@@ -656,6 +656,7 @@ CGameState::CGameState()
 	//objCaller = new CObjectCallersHandler;
 	globalEffects.setDescription("Global effects");
 	globalEffects.setNodeType(CBonusSystemNode::GLOBAL_EFFECTS);
+	day = 0;
 }
 
 CGameState::~CGameState()
@@ -2866,8 +2867,8 @@ CGHeroInstance * CGameState::getUsedHero(HeroTypeID hid) const
 }
 
 PlayerState::PlayerState()
- : color(-1), enteredWinningCheatCode(0),
+ : color(-1), human(false), enteredWinningCheatCode(false),
-   enteredLosingCheatCode(0), status(EPlayerStatus::INGAME)
+   enteredLosingCheatCode(false), status(EPlayerStatus::INGAME)
 {
 	setNodeType(PLAYER);
 }
@@ -2933,6 +2934,7 @@ void InfoAboutArmy::initFromArmy(const CArmedInstance *Army, bool detailed)
 
 void InfoAboutHero::assign(const InfoAboutHero & iah)
 {
+	vstd::clear_pointer(details);
 	InfoAboutArmy::operator = (iah);
 
 	details = (iah.details ? new Details(*iah.details) : nullptr);
@@ -2962,7 +2964,7 @@ InfoAboutHero::InfoAboutHero(const CGHeroInstance *h, InfoAboutHero::EInfoLevel
 
 InfoAboutHero::~InfoAboutHero()
 {
-	delete details;
+	vstd::clear_pointer(details);
 }
 
 InfoAboutHero & InfoAboutHero::operator=(const InfoAboutHero & iah)
@@ -2973,6 +2975,7 @@ InfoAboutHero & InfoAboutHero::operator=(const InfoAboutHero & iah)
 
 void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLevel infoLevel)
 {
+	vstd::clear_pointer(details);
 	if(!h)
 		return;
 
@@ -3013,14 +3016,18 @@ InfoAboutTown::InfoAboutTown():
 
 }
 
-InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed)
+InfoAboutTown::InfoAboutTown(const CGTownInstance *t, bool detailed):
+	details(nullptr),
+	tType(nullptr),
+	built(0),
+	fortLevel(0)
 {
 	initFromTown(t, detailed);
 }
 
 InfoAboutTown::~InfoAboutTown()
 {
-	delete details;
+	vstd::clear_pointer(details);
 }
 
 void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
@@ -3032,6 +3039,8 @@ void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 	name = t->name;
 	tType = t->town;
 
+	vstd::clear_pointer(details);
+
 	if(detailed)
 	{
 		//include details about hero

lib/CGameStateFwd.h

@@ -173,7 +173,9 @@ struct DLL_LINKAGE QuestInfo //universal interface for human and AI
 	const CGObjectInstance * obj; //related object, most likely Seer Hut
 	int3 tile;
 
-	QuestInfo(){};
+	QuestInfo()
+		: quest(nullptr), obj(nullptr), tile(-1,-1,-1)
+	{};
 	QuestInfo (const CQuest * Quest, const CGObjectInstance * Obj, int3 Tile) :
 		quest (Quest), obj (Obj), tile (Tile){};
 

lib/CHeroHandler.cpp

@@ -59,7 +59,7 @@ EAlignment::EAlignment CHeroClass::getAlignment() const
 }
 
 CHeroClass::CHeroClass()
- : commander(nullptr)
+ : faction(0), id(0), affinity(0), defaultTavernChance(0), commander(nullptr)
 {
 }
 

lib/CModHandler.cpp

@@ -486,12 +486,22 @@ JsonNode addMeta(JsonNode config, std::string meta)
 	return config;
 }
 
+CModInfo::CModInfo():
+	checksum(0),
+	enabled(false),
+	validation(PENDING)
+{
+
+}
+
 CModInfo::CModInfo(std::string identifier,const JsonNode & local, const JsonNode & config):
 	identifier(identifier),
 	name(config["name"].String()),
 	description(config["description"].String()),
 	dependencies(config["depends"].convertTo<std::set<std::string> >()),
 	conflicts(config["conflicts"].convertTo<std::set<std::string> >()),
+	checksum(0),
+	enabled(false),
 	validation(PENDING),
 	config(addMeta(config, identifier))
 {
@@ -554,6 +564,10 @@ void CModInfo::loadLocalData(const JsonNode & data)
 
 CModHandler::CModHandler()
 {
+    modules.COMMANDERS = false;
+    modules.STACK_ARTIFACT = false;
+    modules.STACK_EXP = false;
+    modules.MITHRIL = false;
 	for (int i = 0; i < GameConstants::RESOURCE_QUANTITY; ++i)
 	{
 		identifiers.registerObject("core", "resource", GameConstants::RESOURCE_NAMES[i], i);

lib/CModHandler.h

@@ -191,7 +191,7 @@ public:
 
 	JsonNode config;
 
-	CModInfo(){}
+	CModInfo();
 	CModInfo(std::string identifier, const JsonNode & local, const JsonNode & config);
 
 	JsonNode saveLocalData() const;

lib/CPathfinder.cpp

@@ -43,6 +43,11 @@ CPathfinder::CPathfinder(CPathsInfo & _out, CGameState * _gs, const CGHeroInstan
 	assert(hero);
 	assert(hero == getHero(hero->id));
 
+    cp = dp = nullptr;
+    ct = dt = nullptr;
+    ctObj = dtObj = nullptr;
+    destAction = CGPathNode::UNKNOWN;
+
 	out.hero = hero;
 	out.hpos = hero->getPosition(false);
 	if(!isInTheMap(out.hpos)/* || !gs->map->isInTheMap(dest)*/) //check input

lib/CTownHandler.cpp

@@ -26,6 +26,12 @@
 
 const int NAMES_PER_TOWN=16; // number of town names per faction in H3 files. Json can define any number
 
+CBuilding::CBuilding():
+	town(nullptr),mode(BUILD_NORMAL)
+{
+
+}
+
 const std::string & CBuilding::Name() const
 {
 	return name;
@@ -64,6 +70,8 @@ si32 CBuilding::getDistance(BuildingID buildID) const
 CFaction::CFaction()
 {
 	town = nullptr;
+	index = 0;
+	alignment = EAlignment::NEUTRAL;
 }
 
 CFaction::~CFaction()
@@ -72,6 +80,7 @@ CFaction::~CFaction()
 }
 
 CTown::CTown()
+	: faction(nullptr), mageLevel(0), primaryRes(0), moatDamage(0), defaultTavernChance(0)
 {
 
 }

lib/CTownHandler.h

@@ -56,6 +56,8 @@ public:
 		BUILD_GRAIL    // 3 - grail - building reqires grail to be built
 	} mode;
 
+	CBuilding();
+
 	const std::string &Name() const;
 	const std::string &Description() const;
 

lib/CondSh.h

@@ -17,7 +17,6 @@ template <typename T> struct CondSh
 	boost::condition_variable cond;
 	boost::mutex mx;
 
-	CondSh() {}
 	CondSh(T t) : data(t) {}
 
 	// set data

lib/HeroBonus.cpp

@@ -1329,6 +1329,7 @@ bool IPropagator::shouldBeAttached(CBonusSystemNode *dest)
 }
 
 CPropagatorNodeType::CPropagatorNodeType()
+	:nodeType(0)
 {
 
 }
@@ -1349,6 +1350,7 @@ CreatureNativeTerrainLimiter::CreatureNativeTerrainLimiter(int TerrainType)
 }
 
 CreatureNativeTerrainLimiter::CreatureNativeTerrainLimiter()
+	: terrainType(-1)
 {
 
 }
@@ -1366,6 +1368,7 @@ CreatureFactionLimiter::CreatureFactionLimiter(int Faction)
 }
 
 CreatureFactionLimiter::CreatureFactionLimiter()
+	: faction(-1)
 {
 }
 
@@ -1376,6 +1379,7 @@ int CreatureFactionLimiter::limit(const BonusLimitationContext &context) const
 }
 
 CreatureAlignmentLimiter::CreatureAlignmentLimiter()
+	: alignment(-1)
 {
 }
 

lib/NetPacksBase.h

@@ -128,6 +128,7 @@ struct Component
 		h & id & subtype & val & when;
 	}
 	Component()
+		:id(0), subtype(0), val(0), when(0)
 	{
 	}
 	DLL_LINKAGE explicit Component(const CStackBasicDescriptor &stack);

lib/NetPacksLib.cpp

@@ -1688,6 +1688,7 @@ DLL_LINKAGE void ObstaclesRemoved::applyGs(CGameState *gs)
 
 DLL_LINKAGE CatapultAttack::CatapultAttack()
 {
+	attacker = -1;
 }
 
 DLL_LINKAGE CatapultAttack::~CatapultAttack()

lib/VCMI_Lib.cpp

@@ -125,8 +125,6 @@ void LibClasses::init()
 
 	//FIXME: make sure that everything is ok after game restart
 	//TODO: This should be done every time mod config changes
-
-	IS_AI_ENABLED = false;
 }
 
 void LibClasses::clear()
@@ -164,6 +162,7 @@ void LibClasses::makeNull()
 
 LibClasses::LibClasses()
 {
+	IS_AI_ENABLED = false;
 	//init pointers to handlers
 	makeNull();
 }

lib/VCMI_Lib.h

@@ -33,7 +33,7 @@ class DLL_LINKAGE LibClasses
 	void callWhenDeserializing(); //should be called only by serialize !!!
 	void makeNull(); //sets all handler pointers to null
 public:
-	bool IS_AI_ENABLED; //VLC is the only object visible from both CMT and GeniusAI
+	bool IS_AI_ENABLED; //unused?
 
 	const IBonusTypeHandler * getBth() const;
 

lib/filesystem/FileStream.cpp

@@ -229,6 +229,7 @@ bool FileStream::CreateFile(const boost::filesystem::path& filename)
 {
 	FILE* f = do_open(filename.c_str(), CHAR_LITERAL("wb"));
 	bool result = (f != nullptr);
+	if(result)
 		fclose(f);
 	return result;
 }

lib/mapObjects/CBank.cpp

@@ -28,6 +28,8 @@ static std::string & visitedTxt(const bool visited)
 
 CBank::CBank()
 {
+	daycounter = 0;
+	resetDuration = 0;
 }
 
 CBank::~CBank()

lib/mapObjects/CGPandoraBox.cpp

@@ -34,6 +34,13 @@ static void showInfoDialog(const CGHeroInstance* h, const ui32 txtID, const ui16
 	showInfoDialog(playerID,txtID,soundID);
 }
 
+CGPandoraBox::CGPandoraBox()
+	: hasGuardians(false), gainedExp(0), manaDiff(0), moraleDiff(0), luckDiff(0)
+{
+
+}
+
+
 void CGPandoraBox::initObj(CRandomGenerator & rand)
 {
 	blockVisit = (ID==Obj::PANDORAS_BOX); //block only if it's really pandora's box (events also derive from that class)
@@ -362,6 +369,12 @@ void CGPandoraBox::afterSuccessfulVisit() const
 	cb->removeAfterVisit(this);
 }
 
+CGEvent::CGEvent()
+	: CGPandoraBox(), removeAfterVisit(false), availableFor(0), computerActivate(false), humanActivate(false)
+{
+
+}
+
 void CGEvent::onHeroVisit( const CGHeroInstance * h ) const
 {
 	if(!(availableFor & (1 << h->tempOwner.getNum())))

lib/mapObjects/CGPandoraBox.h

@@ -35,7 +35,7 @@ public:
 	std::vector<SpellID> spells; //gained spells
 	CCreatureSet creatures; //gained creatures
 
-	CGPandoraBox() : gainedExp(0), manaDiff(0), moraleDiff(0), luckDiff(0){};
+	CGPandoraBox();
 	void initObj(CRandomGenerator & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	void battleFinished(const CGHeroInstance *hero, const BattleResult &result) const override;
@@ -71,7 +71,7 @@ public:
 		h & removeAfterVisit & availableFor & computerActivate & humanActivate;
 	}
 
-	CGEvent() : CGPandoraBox(){};
+	CGEvent();
 	void onHeroVisit(const CGHeroInstance * h) const override;
 private:
 	void activated(const CGHeroInstance * h) const;

lib/mapObjects/CGTownInstance.cpp

@@ -370,7 +370,6 @@ CGTownInstance::EFortLevel CGTownInstance::fortLevel() const //0 - none, 1 - for
 
 int CGTownInstance::hallLevel() const // -1 - none, 0 - village, 1 - town, 2 - city, 3 - capitol
 {
-
 	if (hasBuilt(BuildingID::CAPITOL))
 		return 3;
 	if (hasBuilt(BuildingID::CITY_HALL))
@@ -381,6 +380,7 @@ int CGTownInstance::hallLevel() const // -1 - none, 0 - village, 1 - town, 2 - c
 		return 0;
 	return -1;
 }
+
 int CGTownInstance::mageGuildLevel() const
 {
 	if (hasBuilt(BuildingID::MAGES_GUILD_5))

lib/mapObjects/CObjectClassesHandler.cpp

@@ -357,6 +357,17 @@ std::string CObjectClassesHandler::getObjectHandlerName(si32 type) const
 	return objects.at(type)->handlerName;
 }
 
+AObjectTypeHandler::AObjectTypeHandler():
+	type(-1), subtype(-1)
+{
+
+}
+
+AObjectTypeHandler::~AObjectTypeHandler()
+{
+
+}
+
 void AObjectTypeHandler::setType(si32 type, si32 subtype)
 {
 	this->type = type;

lib/mapObjects/CObjectClassesHandler.h

@@ -118,8 +118,8 @@ public:
 
 	si32 type;
 	si32 subtype;
-
+	AObjectTypeHandler();
-	virtual ~AObjectTypeHandler(){}
+	virtual ~AObjectTypeHandler();
 
 	void setType(si32 type, si32 subtype);
 	void setTypeName(std::string type, std::string subtype);

lib/mapObjects/CRewardableObject.cpp

@@ -454,6 +454,7 @@ void CRewardableObject::newTurn(CRandomGenerator & rand) const
 CRewardableObject::CRewardableObject():
 	soundID(soundBase::invalid),
 	selectMode(0),
+	visitMode(0),
 	selectedReward(0),
 	resetDuration(0),
 	canRefuse(false)

lib/mapObjects/CommonConstructors.cpp

@@ -87,6 +87,7 @@ void CTownInstanceConstructor::configureObject(CGObjectInstance * object, CRando
 }
 
 CHeroInstanceConstructor::CHeroInstanceConstructor()
+	:heroClass(nullptr)
 {
 
 }
@@ -257,7 +258,9 @@ std::vector<const CCreature *> CDwellingInstanceConstructor::getProducedCreature
 }
 
 CBankInstanceConstructor::CBankInstanceConstructor()
+	: bankResetDuration(0)
 {
+
 }
 
 void CBankInstanceConstructor::initTypeData(const JsonNode & input)

lib/mapObjects/MiscObjects.cpp

@@ -127,7 +127,6 @@ std::string CGCreature::getHoverText(const CGHeroInstance * hero) const
 			ms.addTxt(MetaString::GENERAL_TXT,243);
 			break;
 		default: //decision = cost in gold
-			VLC->generaltexth->allTexts[244];
 			ms << boost::to_string(boost::format(VLC->generaltexth->allTexts[244]) % decision);
 			break;
 		}
@@ -237,7 +236,7 @@ void CGCreature::initObj(CRandomGenerator & rand)
 		}
 	}
 
-	temppower = stacks[SlotID(0)]->count * 1000;
+	temppower = stacks[SlotID(0)]->count * (ui64)1000;
 	refusedJoining = false;
 }
 

lib/mapObjects/MiscObjects.h

@@ -142,7 +142,7 @@ public:
 	EBonusType bonusType;
 	ui16 bonusID; //ID of skill/spell
 
-	CGScholar() : bonusType(EBonusType::RANDOM){};
+	CGScholar() : bonusType(EBonusType::RANDOM),bonusID(0){};
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	void initObj(CRandomGenerator & rand) override;
 	template <typename Handler> void serialize(Handler &h, const int version)

lib/mapping/CMap.cpp

@@ -217,7 +217,9 @@ CMapHeader::~CMapHeader()
 
 }
 
-CMap::CMap() : checksum(0), grailPos(-1, -1, -1), grailRadius(0), terrain(nullptr)
+CMap::CMap()
+	: checksum(0), grailPos(-1, -1, -1), grailRadius(0), terrain(nullptr),
+	guardingCreaturePositions(nullptr)
 {
 	allHeroes.resize(allowedHeroes.size());
 	allowedAbilities = VLC->heroh->getDefaultAllowedAbilities();

lib/rmg/CMapGenerator.cpp

@@ -37,7 +37,9 @@ void CMapGenerator::foreachDirectNeighbour(const int3& pos, std::function<void(i
 
 
 CMapGenerator::CMapGenerator() :
-    zonesTotal(0), monolithIndex(0)
+	mapGenOptions(nullptr), randomSeed(0), editManager(nullptr),
+	zonesTotal(0), tiles(nullptr), prisonsRemaining(0),
+    monolithIndex(0)
 {
 }
 

lib/rmg/CZonePlacer.cpp

@@ -24,7 +24,9 @@ CPlacedZone::CPlacedZone(const CRmgTemplateZone * zone)
 
 }
 
-CZonePlacer::CZonePlacer(CMapGenerator * Gen) : gen(Gen)
+CZonePlacer::CZonePlacer(CMapGenerator * Gen)
+	: width(0), height(0), scaleX(0), scaleY(0), mapSize(0), gravityConstant(0), stiffnessConstant(0),
+	gen(Gen)
 {
 
 }

lib/spells/BattleSpellMechanics.cpp

@@ -370,7 +370,7 @@ ESpellCastProblem::ESpellCastProblem EarthquakeMechanics::canBeCast(const CBattl
 	if(ti.smart)
 	{
 		//if spell targeting is smart, then only attacker can use it
-		if(cb->playerToSide(caster->getOwner()) != 0)
+		if(cb->playerToSide(caster->getOwner()) != BattleSide::ATTACKER)
 			return ESpellCastProblem::NO_APPROPRIATE_TARGET;
 	}
 
@@ -406,7 +406,9 @@ ESpellCastProblem::ESpellCastProblem HypnotizeMechanics::isImmuneByStack(const I
 ///ObstacleMechanics
 ESpellCastProblem::ESpellCastProblem ObstacleMechanics::canBeCast(const CBattleInfoCallback * cb, const SpellTargetingContext & ctx) const
 {
-	ui8 side = cb->playerToSide(ctx.caster->getOwner());
+	const si8 side = cb->playerToSide(ctx.caster->getOwner());
+	if(side < 0)
+		return ESpellCastProblem::INVALID;
 
 	bool hexesOutsideBattlefield = false;
 
@@ -496,7 +498,11 @@ void PatchObstacleMechanics::applyBattleEffects(const SpellCastEnvironment * env
 ESpellCastProblem::ESpellCastProblem LandMineMechanics::canBeCast(const CBattleInfoCallback * cb, const ECastingMode::ECastingMode mode, const ISpellCaster * caster) const
 {
 	//LandMine are useless if enemy has native stack and can see mines, check for LandMine damage immunity is done in general way by CSpell
-	const ui8 otherSide = !cb->playerToSide(caster->getOwner());
+	const si8 playerSide = cb->playerToSide(caster->getOwner());
+	if(playerSide < 0)
+		return ESpellCastProblem::INVALID;
+
+	const si8 otherSide = !playerSide;
 
 	if(cb->battleHasNativeStack(otherSide))
 		return ESpellCastProblem::NO_APPROPRIATE_TARGET;

lib/spells/CDefaultSpellMechanics.cpp

@@ -635,8 +635,10 @@ std::vector<const CStack *> DefaultSpellMechanics::calculateAffectedStacks(const
 {
 	std::set<const CStack* > attackedCres;//std::set to exclude multiple occurrences of two hex creatures
 
-	const ui8 attackerSide = cb->playerToSide(ctx.caster->getOwner()) == 1;
+	const si8 playerSide = cb->playerToSide(ctx.caster->getOwner());
-	auto attackedHexes = rangeInHexes(ctx.destination, ctx.schoolLvl, attackerSide);
+	if(playerSide < 0)
+		return std::vector<const CStack *>();
+	auto attackedHexes = rangeInHexes(ctx.destination, ctx.schoolLvl, playerSide);
 
 	//hackfix for banned creature massive spells
 	if(!ctx.ti.massive && owner->getLevelInfo(ctx.schoolLvl).range == "X")

lib/spells/CSpellHandler.cpp

@@ -95,10 +95,11 @@ CSpell::LevelInfo::~LevelInfo()
 ///CSpell
 CSpell::CSpell():
 	id(SpellID::NONE), level(0),
+	power(0),
 	combatSpell(false), creatureAbility(false),
 	positiveness(ESpellPositiveness::NEUTRAL),
 	defaultProbability(0),
-	isRising(false), isDamage(false), isOffensive(false),
+	isRising(false), isDamage(false), isOffensive(false), isSpecial(true),
 	targetType(ETargetType::NO_TARGET),
 	mechanics(),
 	adventureMechanics()

server/CGameHandler.cpp

@@ -75,7 +75,7 @@ private:
 	mutable CGameHandler * gh;
 };
 
-CondSh<bool> battleMadeAction;
+CondSh<bool> battleMadeAction(false);
 CondSh<BattleResult *> battleResult(nullptr);
 template <typename T> class CApplyOnGH;
 
@@ -947,12 +947,14 @@ void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &
 				{
 					logGlobal->error("Received a null package marked as request %d from player %d", requestID, player);
 				}
-
+				else
+				{
 					packType = typeList.getTypeID(pack); //get the id of type
 
 					logGlobal->trace("Received client message (request %d by player %d (%s)) of type with ID=%d (%s).\n",
 									 requestID, player, player.getStr(), packType, typeid(*pack).name());
 				}
+			}
 
 			//prepare struct informing that action was applied
 			auto sendPackageResponse = [&](bool succesfullyApplied)
@@ -2148,7 +2150,7 @@ bool CGameHandler::teleportHero(ObjectInstanceID hid, ObjectInstanceID dstid, ui
 	const CGTownInstance *t = getTown(dstid);
 
 	if (!h || !t || h->getOwner() != gs->currentPlayer)
-		logGlobal->error("Invalid call to teleportHero!");
+		COMPLAIN_RET("Invalid call to teleportHero!");
 
 	const CGTownInstance *from = h->visitedTown;
 	if (((h->getOwner() != t->getOwner())
@@ -2173,11 +2175,11 @@ void CGameHandler::setOwner(const CGObjectInstance * obj, PlayerColor owner)
 	std::set<PlayerColor> playerColors = {owner, oldOwner};
 	checkVictoryLossConditions(playerColors);
 
-	if (dynamic_cast<const CGTownInstance *>(obj)) //town captured
+	const CGTownInstance * town = dynamic_cast<const CGTownInstance *>(obj);
+	if (town) //town captured
 	{
 		if (owner < PlayerColor::PLAYER_LIMIT) //new owner is real player
 		{
-			const CGTownInstance * town = dynamic_cast<const CGTownInstance *>(obj);
 			if (town->hasBuilt(BuildingID::PORTAL_OF_SUMMON, ETownType::DUNGEON))
 				setPortalDwelling(town, true, false);
 		}
@@ -2238,6 +2240,7 @@ void CGameHandler::giveResources(PlayerColor player, TResources resources)
 	sr.abs = false;
 	sr.player = player;
 	sr.res = resources;
+	sendAndApply(&sr);
 }
 
 void CGameHandler::giveCreatures(const CArmedInstance *obj, const CGHeroInstance * h, const CCreatureSet &creatures, bool remove)
@@ -3327,6 +3330,9 @@ bool CGameHandler::buyArtifact(ObjectInstanceID hid, ArtifactID aid)
 
 bool CGameHandler::buyArtifact(const IMarket *m, const CGHeroInstance *h, Res::ERes rid, ArtifactID aid)
 {
+	if(!h)
+		COMPLAIN_RET("Only hero can buy artifacts!");
+
 	if (!vstd::contains(m->availableItemsIds(EMarketMode::RESOURCE_ARTIFACT), aid))
 		COMPLAIN_RET("That artifact is unavailable!");
 
@@ -3374,11 +3380,10 @@ bool CGameHandler::buyArtifact(const IMarket *m, const CGHeroInstance *h, Res::E
 
 bool CGameHandler::sellArtifact(const IMarket *m, const CGHeroInstance *h, ArtifactInstanceID aid, Res::ERes rid)
 {
+	COMPLAIN_RET_FALSE_IF((!h), "Only hero can sell artifacts!");
 	const CArtifactInstance *art = h->getArtByInstanceId(aid);
-	if (!art)
+	COMPLAIN_RET_FALSE_IF((!art), "There is no artifact to sell!");
-		COMPLAIN_RET("There is no artifact to sell!");
+	COMPLAIN_RET_FALSE_IF((!art->artType->isTradable()), "Cannot sell a war machine or spellbook!");
-	if (!art->artType->isTradable())
-		COMPLAIN_RET("Cannot sell a war machine or spellbook!");
 
 	int resVal = 0, dump = 1;
 	m->getOffer(art->artType->id, rid, dump, resVal, EMarketMode::ARTIFACT_RESOURCE);
@@ -3388,14 +3393,6 @@ bool CGameHandler::sellArtifact(const IMarket *m, const CGHeroInstance *h, Artif
 	return true;
 }
 
-//void CGameHandler::lootArtifacts (TArtHolder source, TArtHolder dest, std::vector<ui32> &arts)
-//{
-//	//const CGHeroInstance * h1 = dynamic_cast<CGHeroInstance *> source;
-//	//auto s = boost::apply_visitor(GetArtifactSetPtr(), source);
-//	{
-//	}
-//}
-
 bool CGameHandler::buySecSkill(const IMarket *m, const CGHeroInstance *h, SecondarySkill skill)
 {
 	if (!h)
@@ -3445,6 +3442,8 @@ bool CGameHandler::tradeResources(const IMarket *market, ui32 val, PlayerColor p
 
 bool CGameHandler::sellCreatures(ui32 count, const IMarket *market, const CGHeroInstance * hero, SlotID slot, Res::ERes resourceID)
 {
+	if(!hero)
+		COMPLAIN_RET("Only hero can sell creatures!");
 	if (!vstd::contains(hero->Slots(), slot))
 		COMPLAIN_RET("Hero doesn't have any creature in that slot!");
 
@@ -4488,6 +4487,7 @@ void CGameHandler::handleDamageFromObstacle(const CObstacleInstance &obstacle, c
 
 	//helper info
 	const SpellCreatedObstacle *spellObstacle = dynamic_cast<const SpellCreatedObstacle*>(&obstacle); //not nice but we may need spell params
+
 	const ui8 side = !curStack->attackerOwned; //if enemy is defending (false = 0), side of enemy hero is 1 (true)
 	const CGHeroInstance *hero = gs->curB->battleGetFightingHero(side);//FIXME: there may be no hero - landmines in Tower
 
@@ -4497,6 +4497,7 @@ void CGameHandler::handleDamageFromObstacle(const CObstacleInstance &obstacle, c
 	}
 	else if (obstacle.obstacleType == CObstacleInstance::LAND_MINE)
 	{
+		COMPLAIN_RET_IF((!spellObstacle), "Invalid obstacle instance");
 		//You don't get hit by a Mine you can see.
 		if (gs->curB->battleIsObstacleVisibleForSide(obstacle, (BattlePerspective::BattlePerspective)side))
 			return;
@@ -4516,6 +4517,7 @@ void CGameHandler::handleDamageFromObstacle(const CObstacleInstance &obstacle, c
 	}
 	else if (obstacle.obstacleType == CObstacleInstance::FIRE_WALL)
 	{
+		COMPLAIN_RET_IF((!spellObstacle), "Invalid obstacle instance");
 		const CSpell * sp = SpellID(SpellID::FIRE_WALL).toSpell();
 
 		if (sp->isImmuneByStack(hero, curStack))
@@ -5250,6 +5252,9 @@ void CGameHandler::visitObjectOnTile(const TerrainTile &t, const CGHeroInstance
 
 bool CGameHandler::sacrificeCreatures(const IMarket *market, const CGHeroInstance *hero, SlotID slot, ui32 count)
 {
+	if (!hero)
+		COMPLAIN_RET("You need hero to sacrifice creature!");
+
 	int oldCount = hero->getStackCount(slot);
 
 	if (oldCount < count)
@@ -5271,15 +5276,21 @@ bool CGameHandler::sacrificeCreatures(const IMarket *market, const CGHeroInstanc
 
 bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, ArtifactPosition slot)
 {
+	if (!hero)
+		COMPLAIN_RET("You need hero to sacrifice artifact!");
+
 	ArtifactLocation al(hero, slot);
 	const CArtifactInstance *a = al.getArt();
 
-	if (!a)
+	COMPLAIN_RET_FALSE_IF(!a,"Cannot find artifact to sacrifice!");
-		COMPLAIN_RET("Cannot find artifact to sacrifice!");
-
 
 	int dmp, expToGive;
-	m->getOffer(hero->getArtTypeId(slot), 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
+	const CArtifactInstance * art = hero->getArt(slot);
+	COMPLAIN_RET_FALSE_IF((!art), "No artifact at position to sacrifice!");
+
+	si32 typId = art->artType->id;
+
+	m->getOffer(typId, 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
 
 	removeArtifact(al);
 	changePrimSkill(hero, PrimarySkill::EXPERIENCE, expToGive);
@@ -6275,6 +6286,8 @@ CGameHandler::FinishingBattleHelper::FinishingBattleHelper(std::shared_ptr<const
 CGameHandler::FinishingBattleHelper::FinishingBattleHelper()
 {
 	winnerHero = loserHero = nullptr;
+	duel = false;
+	remainingBattleQueriesCount = 0;
 }
 
 CRandomGenerator & CGameHandler::getRandomGenerator()

server/CGameHandler.h

@@ -83,7 +83,6 @@ public:
 	enum EVisitDest {VISIT_DEST, DONT_VISIT_DEST};
 	enum ELEaveTile {LEAVING_TILE, REMAINING_ON_TILE};
 
-	CVCMIServer *s;
 	std::map<PlayerColor, CConnection*> connections; //player color -> connection to client with interface of that player
 	PlayerStatuses states; //player color -> player state
 	std::set<CConnection*> conns;

server/CQuery.cpp

@@ -239,8 +239,9 @@ CBattleQuery::CBattleQuery(const BattleInfo *Bi)
 }
 
 CBattleQuery::CBattleQuery()
+	:bi(nullptr)
 {
-
+	belligerents[0] = belligerents[1] = nullptr;
 }
 
 bool CBattleQuery::blocksPack(const CPack *pack) const
@@ -323,7 +324,10 @@ void CTeleportDialogQuery::notifyObjectAboutRemoval(const CObjectVisitQuery &obj
 {
 	// do not change to dynamic_ptr_cast - SIGSEGV!
 	auto obj = dynamic_cast<const CGTeleport*>(objectVisit.visitedObject);
+	if(obj)
 		obj->teleportDialogAnswered(objectVisit.visitingHero, *answer, td.exits);
+	else
+		logGlobal->error("Invalid instance in teleport query");
 }
 
 CTeleportDialogQuery::CTeleportDialogQuery(const TeleportDialog &td)

server/NetPacksServer.cpp

@@ -177,16 +177,10 @@ bool TradeOnMarketplace::applyGh( CGameHandler *gh )
 	case EMarketMode::RESOURCE_PLAYER:
 		return gh->sendResources(val, player, static_cast<Res::ERes>(r1), PlayerColor(r2));
 	case EMarketMode::CREATURE_RESOURCE:
-		if(!hero)
-			COMPLAIN_AND_RETURN("Only hero can sell creatures!");
 		return gh->sellCreatures(val, m, hero, SlotID(r1), static_cast<Res::ERes>(r2));
 	case EMarketMode::RESOURCE_ARTIFACT:
-		if(!hero)
-			COMPLAIN_AND_RETURN("Only hero can buy artifacts!");
 		return gh->buyArtifact(m, hero, static_cast<Res::ERes>(r1), ArtifactID(r2));
 	case EMarketMode::ARTIFACT_RESOURCE:
-		if(!hero)
-			COMPLAIN_AND_RETURN("Only hero can sell artifacts!");
 		return gh->sellArtifact(m, hero, ArtifactInstanceID(r1), static_cast<Res::ERes>(r2));
 	case EMarketMode::CREATURE_UNDEAD:
 		return gh->transformInUndead(m, hero, SlotID(r1));