Merge pull request #5295 from IvanSavenko/bugfixing

[1.6.4] Bugfixing for recently reported issues

AI/BattleAI/BattleEvaluator.cpp

@@ -390,7 +390,7 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, const Battl
 			return reachability.distances[h1.toInt()] < reachability.distances[h2.toInt()];
 		});
 
-	BattleHex bestNeighbour = hexes.front();
+	BattleHex bestNeighbour = targetHexes.front();
 
 	if(reachability.distances[bestNeighbour.toInt()] > GameConstants::BFIELD_SIZE)
 	{

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -1162,7 +1162,7 @@ public:
 		Goals::BuildThis & buildThis = dynamic_cast<Goals::BuildThis &>(*task);
 		auto & bi = buildThis.buildingInfo;
 		
-		evaluationContext.goldReward += 7 * bi.dailyIncome[EGameResID::GOLD] / 2; // 7 day income but half we already have
+		evaluationContext.goldReward += 7 * bi.dailyIncome.marketValue() / 2; // 7 day income but half we already have
 		evaluationContext.heroRole = HeroRole::MAIN;
 		evaluationContext.movementCostByRole[evaluationContext.heroRole] += bi.prerequisitesCount;
 		int32_t cost = bi.buildCost[EGameResID::GOLD];

client/CPlayerInterface.cpp

@@ -182,6 +182,9 @@ void CPlayerInterface::closeAllDialogs()
 		if(infoWindow && infoWindow->ID != QueryID::NONE)
 			break;
 
+		if (topWindow == nullptr)
+			throw std::runtime_error("Invalid or non-existing top window! Total windows: " + std::to_string(GH.windows().count()));
+
 		topWindow->close();
 	}
 }

client/gui/CIntObject.cpp

@@ -343,6 +343,9 @@ WindowBase::WindowBase(int used_, Point pos_)
 void WindowBase::close()
 {
 	if(!GH.windows().isTopWindow(this))
-		throw std::runtime_error("Only top interface can be closed");
+	{
+		auto topWindow = GH.windows().topWindow<IShowActivatable>().get();
+		throw std::runtime_error(std::string("Only top interface can be closed! Top window is ") + typeid(*this).name() + " but attempted to close " + typeid(*topWindow).name());
+	}
 	GH.windows().popWindows(1);
 }

client/mainmenu/CHighScoreScreen.cpp

@@ -274,7 +274,7 @@ void CHighScoreInputScreen::show(Canvas & to)
 
 void CHighScoreInputScreen::clickPressed(const Point & cursorPosition)
 {
-	if(statisticButton->pos.isInside(cursorPosition))
+	if(statisticButton && statisticButton->pos.isInside(cursorPosition))
 		return;
 
 	OBJECT_CONSTRUCTION;
@@ -288,7 +288,7 @@ void CHighScoreInputScreen::clickPressed(const Point & cursorPosition)
 	if(!input)
 	{
 		input = std::make_shared<CHighScoreInput>(calc.parameters[0].playerName,
-		[&] (std::string text) 
+		[&] (std::string text)
 		{
 			if(!text.empty())
 			{

client/render/CAnimation.cpp

@@ -212,10 +212,17 @@ void CAnimation::createFlippedGroup(const size_t sourceGroup, const size_t targe
 
 ImageLocator CAnimation::getImageLocator(size_t frame, size_t group) const
 {
-	const ImageLocator & locator = source.at(group).at(frame);
+	try
+	{
+		const ImageLocator & locator = source.at(group).at(frame);
 
-	if (!locator.empty())
-		return locator;
+		if (!locator.empty())
+			return locator;
+	}
+	catch (std::out_of_range &)
+	{
+		throw std::runtime_error("Frame " + std::to_string(frame) + " of group " + std::to_string(group) + " is missing from animation " + name.getOriginalName() );
+	}
 
 	return ImageLocator(name, frame, group);
 }

client/windows/CCreatureWindow.cpp

@@ -831,12 +831,18 @@ void CStackWindow::init()
 
 void CStackWindow::initBonusesList()
 {
+	auto inputPtr = info->stackNode->getBonuses(CSelector(Bonus::Permanent), Selector::all);
+
 	BonusList output;
-	BonusList input;
-	input = *(info->stackNode->getBonuses(CSelector(Bonus::Permanent), Selector::all));
+	BonusList input = *inputPtr;
+
 	std::sort(input.begin(), input.end(), [this](std::shared_ptr<Bonus> v1, std::shared_ptr<Bonus> & v2){
 		if (v1->source != v2->source)
-			return v1->source == BonusSource::CREATURE_ABILITY || (v1->source < v2->source);
+		{
+			int priorityV1 = v1->source == BonusSource::CREATURE_ABILITY ? -1 : static_cast<int>(v1->source);
+			int priorityV2 = v2->source == BonusSource::CREATURE_ABILITY ? -1 : static_cast<int>(v2->source);
+			return priorityV1 < priorityV2;
+		}
 		else
 			return  info->stackNode->bonusToString(v1, false) < info->stackNode->bonusToString(v2, false);
 	});

clientapp/EntryPoint.cpp

@@ -94,7 +94,7 @@ static void init()
 
 	// Debug code to load all maps on start
 	//ClientCommandManager commandController;
-	//commandController.processCommand("convert txt", false);
+	//commandController.processCommand("translate maps", false);
 }
 
 static void checkForModLoadingFailure()

docs/modders/Entities_Format/Artifact_Format.md

@@ -68,7 +68,12 @@ In order to make functional artifact you also need:
 		"artifact3"
 	],
 	
-	// Optional, by default is false. Set to true if components are supposed to be fused. 
+    // Optional, by default is false. Set to true if components are supposed to be fused.
+	// When artifact is fused, all its components are removed and hero receives fused artifact in their place.
+	// As result of this, fused artifact:
+	// - can not be disassembled
+	// - unlike combined artifacts, fused artifact does not locks slots of its components
+	// - does not inherits bonuses from its constituent parts
 	"fusedComponents" : true,
 
 	// Creature id to use on battle field. If set, this artifact is war machine

launcher/modManager/cmodlistview_moc.cpp

@@ -955,38 +955,38 @@ void CModListView::on_tabWidget_currentChanged(int index)
 
 void CModListView::loadScreenshots()
 {
-	if(ui->tabWidget->currentIndex() == 2)
+	if(ui->tabWidget->currentIndex() != 2)
+		return;
+
+	assert(ui->allModsView->currentIndex().isValid());
+
+
+	if (!ui->allModsView->currentIndex().isValid())
+		return;
+		
+	ui->screenshotsList->clear();
+	QString modName = ui->allModsView->currentIndex().data(ModRoles::ModNameRole).toString();
+	assert(modStateModel->isModExists(modName)); //should be filtered out by check above
+
+	for(QString url : modStateModel->getMod(modName).getScreenshots())
 	{
-		if(!ui->allModsView->currentIndex().isValid())
+		// URL must be encoded to something else to get rid of symbols illegal in file names
+		const auto hashed = QCryptographicHash::hash(url.toUtf8(), QCryptographicHash::Md5);
+		const auto fileName = QString{QLatin1String{"%1.png"}}.arg(QLatin1String{hashed.toHex()});
+
+		const auto fullPath = QString{QLatin1String{"%1/%2"}}.arg(CLauncherDirs::downloadsPath(), fileName);
+		QPixmap pixmap(fullPath);
+		if(pixmap.isNull())
 		{
-			// select the first mod, so we can access its data
-			ui->allModsView->setCurrentIndex(filterModel->index(0, 0));
+			// image file not exists or corrupted - try to redownload
+			downloadFile(fileName, url, tr("screenshots"));
 		}
-		
-		ui->screenshotsList->clear();
-		QString modName = ui->allModsView->currentIndex().data(ModRoles::ModNameRole).toString();
-		assert(modStateModel->isModExists(modName)); //should be filtered out by check above
-
-		for(QString url : modStateModel->getMod(modName).getScreenshots())
+		else
 		{
-			// URL must be encoded to something else to get rid of symbols illegal in file names
-			const auto hashed = QCryptographicHash::hash(url.toUtf8(), QCryptographicHash::Md5);
-			const auto fileName = QString{QLatin1String{"%1.png"}}.arg(QLatin1String{hashed.toHex()});
-
-			const auto fullPath = QString{QLatin1String{"%1/%2"}}.arg(CLauncherDirs::downloadsPath(), fileName);
-			QPixmap pixmap(fullPath);
-			if(pixmap.isNull())
-			{
-				// image file not exists or corrupted - try to redownload
-				downloadFile(fileName, url, tr("screenshots"));
-			}
-			else
-			{
-				// managed to load cached image
-				QIcon icon(pixmap);
-				auto * item = new QListWidgetItem(icon, QString(tr("Screenshot %1")).arg(ui->screenshotsList->count() + 1));
-				ui->screenshotsList->addItem(item);
-			}
+			// managed to load cached image
+			QIcon icon(pixmap);
+			auto * item = new QListWidgetItem(icon, QString(tr("Screenshot %1")).arg(ui->screenshotsList->count() + 1));
+			ui->screenshotsList->addItem(item);
 		}
 	}
 }

launcher/modManager/modstatecontroller.cpp

@@ -148,6 +148,9 @@ bool ModStateController::canUninstallMod(QString modname)
 
 bool ModStateController::canEnableMod(QString modname)
 {
+	if (!modList->isModExists(modname))
+		return false;
+
 	auto mod = modList->getMod(modname);
 
 	if(modList->isModEnabled(modname))

launcher/startGame/StartGameTab.cpp

@@ -468,7 +468,7 @@ void StartGameTab::on_buttonPresetRename_clicked()
 			currentName,
 			&ok);
 
-		if (ok && !newName.isEmpty())
+		if (ok && !newName.isEmpty() && newName != currentName)
 		{
 			getMainWindow()->getModView()->renamePreset(currentName, newName);
 			refreshPresets();

lib/ResourceSet.cpp

@@ -67,7 +67,11 @@ void ResourceSet::positive()
 void ResourceSet::applyHandicap(int percentage)
 {
 	for(auto & elem : *this)
-		elem = vstd::divideAndCeil(elem * percentage, 100);
+	{
+		int64_t newAmount = vstd::divideAndCeil(static_cast<int64_t>(elem) * percentage, 100);
+		int64_t cap = GameConstants::PLAYER_RESOURCES_CAP;
+		elem = std::min(cap, newAmount);
+	}
 }
 
 static bool canAfford(const ResourceSet &res, const ResourceSet &price)

lib/battle/CBattleInfoCallback.cpp

@@ -1717,6 +1717,8 @@ battle::Units CBattleInfoCallback::battleAdjacentUnits(const battle::Unit * unit
 
 	const auto & units = battleGetUnitsIf([=](const battle::Unit * unit)
 	{
+		if (unit->isDead())
+			return false;
 		const auto & unitHexes = unit->getHexes();
 		for (const auto & hex : unitHexes)
 			if (hexes.contains(hex))

lib/battle/CUnitState.cpp

@@ -525,7 +525,7 @@ bool CUnitState::isCaster() const
 
 bool CUnitState::canShootBlocked() const
 {
-	return bonusCache.getBonusValue(UnitBonusValuesProxy::HAS_FREE_SHOOTING);
+	return bonusCache.hasBonus(UnitBonusValuesProxy::HAS_FREE_SHOOTING);
 }
 
 bool CUnitState::canShoot() const

lib/gameState/CGameState.cpp

@@ -884,41 +884,45 @@ void CGameState::initTowns()
 		//init spells
 		vti->spells.resize(GameConstants::SPELL_LEVELS);
 		vti->possibleSpells -= SpellID::PRESET;
+
 		for(ui32 z=0; z<vti->obligatorySpells.size();z++)
 		{
 			const auto * s = vti->obligatorySpells[z].toSpell();
 			vti->spells[s->getLevel()-1].push_back(s->id);
 			vti->possibleSpells -= s->id;
 		}
-		while(!vti->possibleSpells.empty())
+
+		vstd::erase_if(vti->possibleSpells, [&](const SpellID & spellID)
 		{
-			ui32 total=0;
-			int sel = -1;
+			const auto * spell = spellID.toSpell();
 
-			for(ui32 ps=0;ps<vti->possibleSpells.size();ps++)
-				total += vti->possibleSpells[ps].toSpell()->getProbability(vti->getFactionID());
+			if (spell->getProbability(vti->getFactionID()) == 0)
+				return true;
 
-			if (total == 0) // remaining spells have 0 probability
-				break;
+			if (spell->isSpecial() || spell->isCreatureAbility())
+				return true;
+
+			if (!isAllowed(spellID))
+				return true;
+
+			return false;
+		});
+
+		std::vector<int> spellWeights;
+		for (auto & spellID : vti->possibleSpells)
+			spellWeights.push_back(spellID.toSpell()->getProbability(vti->getFactionID()));
 
-			auto r = getRandomGenerator().nextInt(total - 1);
-			for(ui32 ps=0; ps<vti->possibleSpells.size();ps++)
-			{
-				r -= vti->possibleSpells[ps].toSpell()->getProbability(vti->getFactionID());
-				if(r<0)
-				{
-					sel = ps;
-					break;
-				}
-			}
-			if(sel<0)
-				sel=0;
 
-			const auto * s = vti->possibleSpells[sel].toSpell();
+		while(!vti->possibleSpells.empty())
+		{
+			size_t index = RandomGeneratorUtil::nextItemWeighted(spellWeights, getRandomGenerator());
+
+			const auto * s = vti->possibleSpells[index].toSpell();
 			vti->spells[s->getLevel()-1].push_back(s->id);
-			vti->possibleSpells -= s->id;
+
+			vti->possibleSpells.erase(vti->possibleSpells.begin() + index);
+			spellWeights.erase(spellWeights.begin() + index);
 		}
-		vti->possibleSpells.clear();
 	}
 }
 

lib/mapObjects/CGTownInstance.cpp

@@ -394,8 +394,18 @@ void CGTownInstance::initializeConfigurableBuildings(vstd::RNG & rand)
 {
 	for(const auto & kvp : getTown()->buildings)
 	{
-		if(!kvp.second->rewardableObjectInfo.getParameters().isNull())
+		if(kvp.second->rewardableObjectInfo.getParameters().isNull())
+			continue;
+
+		try {
 			rewardableBuildings[kvp.first] = new TownRewardableBuildingInstance(this, kvp.second->bid, rand);
+		}
+		catch (std::runtime_error & e)
+		{
+			std::string buildingConfig = kvp.second->rewardableObjectInfo.getParameters().toCompactString();
+			std::replace(buildingConfig.begin(), buildingConfig.end(), '\n', ' ');
+			throw std::runtime_error("Failed to load rewardable building data for " + kvp.second->getJsonKey() + " Reason: " + e.what() + ", config was: " + buildingConfig);
+		}
 	}
 }
 

lib/mapObjects/TownBuildingInstance.cpp

@@ -67,11 +67,6 @@ TownRewardableBuildingInstance::TownRewardableBuildingInstance(IGameCallback *cb
 
 TownRewardableBuildingInstance::TownRewardableBuildingInstance(CGTownInstance * town, const BuildingID & index, vstd::RNG & rand)
 	: TownBuildingInstance(town, index)
-{
-	initObj(rand);
-}
-
-void TownRewardableBuildingInstance::initObj(vstd::RNG & rand)
 {
 	assert(town && town->getTown());
 	configuration = generateConfiguration(rand);

lib/mapObjects/TownBuildingInstance.h

@@ -81,8 +81,6 @@ public:
 	/// gives second part of reward after hero level-ups for proper granting of spells/mana
 	void heroLevelUpDone(const CGHeroInstance *hero) const override;
 	
-	void initObj(vstd::RNG & rand) override;
-	
 	/// applies player selection of reward
 	void blockingDialogAnswered(const CGHeroInstance *hero, int32_t answer) const override;
 	

lib/modding/IdentifierStorage.cpp

@@ -500,7 +500,7 @@ std::vector<std::string> CIdentifierStorage::getModsWithFailedRequests() const
 	std::vector<std::string> result;
 
 	for (const auto & request : failedRequests)
-		if (!vstd::contains(result, request.localScope) && ModScope::isScopeReserved(request.localScope))
+		if (!vstd::contains(result, request.localScope) && !ModScope::isScopeReserved(request.localScope))
 			result.push_back(request.localScope);
 
 	return result;

lib/serializer/BinarySerializer.h

@@ -301,28 +301,25 @@ public:
 	template <typename T>
 	void save(const std::set<T> &data)
 	{
-		auto & d = const_cast<std::set<T> &>(data);
-		uint32_t length = d.size();
+		uint32_t length = data.size();
 		save(length);
-		for(auto i = d.begin(); i != d.end(); i++)
+		for(auto i = data.begin(); i != data.end(); i++)
 			save(*i);
 	}
 	template <typename T, typename U>
 	void save(const std::unordered_set<T, U> &data)
 	{
-		auto & d = const_cast<std::unordered_set<T, U> &>(data);
-		uint32_t length = d.size();
+		uint32_t length = data.size();
 		*this & length;
-		for(auto i = d.begin(); i != d.end(); i++)
+		for(auto i = data.begin(); i != data.end(); i++)
 			save(*i);
 	}
 	template <typename T>
 	void save(const std::list<T> &data)
 	{
-		auto & d = const_cast<std::list<T> &>(data);
-		uint32_t length = d.size();
+		uint32_t length = data.size();
 		*this & length;
-		for(auto i = d.begin(); i != d.end(); i++)
+		for(auto i = data.begin(); i != data.end(); i++)
 			save(*i);
 	}
 

server/CGameHandler.cpp

@@ -3517,8 +3517,6 @@ void CGameHandler::checkVictoryLossConditionsForPlayer(PlayerColor player)
 		addStatistics(peg.statistic); // add last turn befor win / loss
 		sendAndApply(peg);
 
-		turnOrder->onPlayerEndsGame(player);
-
 		if (victoryLossCheckResult.victory())
 		{
 			//one player won -> all enemies lost
@@ -3546,6 +3544,9 @@ void CGameHandler::checkVictoryLossConditionsForPlayer(PlayerColor player)
 		}
 		else
 		{
+			// give turn to next player(s)
+			turnOrder->onPlayerEndsGame(player);
+
 			//copy heroes vector to avoid iterator invalidation as removal change PlayerState
 			auto hlp = p->getHeroes();
 			for (auto h : hlp) //eliminate heroes

server/processors/NewTurnProcessor.cpp

@@ -258,13 +258,14 @@ ResourceSet NewTurnProcessor::generatePlayerIncome(PlayerColor playerID, bool ne
 		for (GameResID i : GameResID::ALL_RESOURCES())
 		{
 			const std::string & name = GameConstants::RESOURCE_NAMES[i];
-			int weeklyBonus = difficultyConfig[name].Integer();
-			int dayOfWeek = gameHandler->gameState()->getDate(Date::DAY_OF_WEEK);
-			int dailyIncome = incomeHandicapped[i];
-			int amountTillToday = dailyIncome * weeklyBonus * (dayOfWeek-1) / 7 / 100;
-			int amountAfterToday = dailyIncome * weeklyBonus * dayOfWeek / 7 / 100;
-			int dailyBonusToday = amountAfterToday - amountTillToday;
-			incomeHandicapped[static_cast<GameResID>(i)] += dailyBonusToday;
+			int64_t weeklyBonus = difficultyConfig[name].Integer();
+			int64_t dayOfWeek = gameHandler->gameState()->getDate(Date::DAY_OF_WEEK);
+			int64_t dailyIncome = incomeHandicapped[i];
+			int64_t amountTillToday = dailyIncome * weeklyBonus * (dayOfWeek-1) / 7 / 100;
+			int64_t amountAfterToday = dailyIncome * weeklyBonus * dayOfWeek / 7 / 100;
+			int64_t dailyBonusToday = amountAfterToday - amountTillToday;
+			int64_t totalIncomeToday = std::min(GameConstants::PLAYER_RESOURCES_CAP, incomeHandicapped[i] + dailyBonusToday);
+			incomeHandicapped[i] = totalIncomeToday;
 		}
 	}