Merge pull request #5111 from IvanSavenko/bugfixing_beta_4

Final bugfixes for 1.6.0

client/render/CBitmapHandler.cpp

@@ -12,6 +12,7 @@
 
 #include "../renderSDL/SDL_Extensions.h"
 
+#include "../lib/ExceptionsCommon.h"
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/vcmi_endian.h"
 
@@ -112,40 +113,47 @@ SDL_Surface * BitmapHandler::loadBitmapFromDir(const ImagePath & path)
 
 	SDL_Surface * ret=nullptr;
 
-	auto readFile = CResourceHandler::get()->load(path)->readAll();
+	try {
+		auto readFile = CResourceHandler::get()->load(path)->readAll();
 
-	if (isPCX(readFile.first.get()))
-	{//H3-style PCX
-		ret = loadH3PCX(readFile.first.get(), readFile.second);
-		if (!ret)
-		{
-			logGlobal->error("Failed to open %s as H3 PCX!", path.getOriginalName());
-			return nullptr;
-		}
-	}
-	else
-	{ //loading via SDL_Image
-		ret = IMG_Load_RW(
-				  //create SDL_RW with our data (will be deleted by SDL)
-				  SDL_RWFromConstMem((void*)readFile.first.get(), (int)readFile.second),
-				  1); // mark it for auto-deleting
-		if (ret)
-		{
-			if (ret->format->palette)
+		if (isPCX(readFile.first.get()))
+		{//H3-style PCX
+			ret = loadH3PCX(readFile.first.get(), readFile.second);
+			if (!ret)
 			{
-				// set correct value for alpha\unused channel
-				// NOTE: might be unnecessary with SDL2
-				for (int i=0; i < ret->format->palette->ncolors; i++)
-					ret->format->palette->colors[i].a = SDL_ALPHA_OPAQUE;
+				logGlobal->error("Failed to open %s as H3 PCX!", path.getOriginalName());
+				return nullptr;
 			}
 		}
 		else
-		{
-			logGlobal->error("Failed to open %s via SDL_Image", path.getOriginalName());
-			logGlobal->error("Reason: %s", IMG_GetError());
-			return nullptr;
+		{ //loading via SDL_Image
+			ret = IMG_Load_RW(
+					  //create SDL_RW with our data (will be deleted by SDL)
+					  SDL_RWFromConstMem((void*)readFile.first.get(), (int)readFile.second),
+					  1); // mark it for auto-deleting
+			if (ret)
+			{
+				if (ret->format->palette)
+				{
+					// set correct value for alpha\unused channel
+					// NOTE: might be unnecessary with SDL2
+					for (int i=0; i < ret->format->palette->ncolors; i++)
+						ret->format->palette->colors[i].a = SDL_ALPHA_OPAQUE;
+				}
+			}
+			else
+			{
+				logGlobal->error("Failed to open %s via SDL_Image", path.getOriginalName());
+				logGlobal->error("Reason: %s", IMG_GetError());
+				return nullptr;
+			}
 		}
 	}
+	catch (const DataLoadingException & e)
+	{
+		logGlobal->error("%s", e.what());
+		return nullptr;
+	}
 
 	// When modifying anything here please check two use cases:
 	// 1) Vampire mansion in Necropolis (not 1st color is transparent)

client/render/IImage.h

@@ -111,12 +111,12 @@ public:
 	virtual bool isTransparent(const Point & coords) const = 0;
 	virtual void draw(SDL_Surface * where, SDL_Palette * palette, const Point & dest, const Rect * src, const ColorRGBA & colorMultiplier, uint8_t alpha, EImageBlitMode mode) const = 0;
 
-	virtual std::shared_ptr<IImage> createImageReference(EImageBlitMode mode) const = 0;
+	[[nodiscard]] virtual std::shared_ptr<IImage> createImageReference(EImageBlitMode mode) const = 0;
 
-	virtual std::shared_ptr<const ISharedImage> horizontalFlip() const = 0;
-	virtual std::shared_ptr<const ISharedImage> verticalFlip() const = 0;
-	virtual std::shared_ptr<const ISharedImage> scaleInteger(int factor, SDL_Palette * palette, EImageBlitMode blitMode) const = 0;
-	virtual std::shared_ptr<const ISharedImage> scaleTo(const Point & size, SDL_Palette * palette) const = 0;
+	[[nodiscard]] virtual std::shared_ptr<const ISharedImage> horizontalFlip() const = 0;
+	[[nodiscard]] virtual std::shared_ptr<const ISharedImage> verticalFlip() const = 0;
+	[[nodiscard]] virtual std::shared_ptr<const ISharedImage> scaleInteger(int factor, SDL_Palette * palette, EImageBlitMode blitMode) const = 0;
+	[[nodiscard]] virtual std::shared_ptr<const ISharedImage> scaleTo(const Point & size, SDL_Palette * palette) const = 0;
 
 
 	virtual ~ISharedImage() = default;

client/renderSDL/ImageScaled.cpp

@@ -43,6 +43,9 @@ void ImageScaled::scaleInteger(int factor)
 
 void ImageScaled::scaleTo(const Point & size)
 {
+	if (source)
+		source = source->scaleTo(size, nullptr);
+
 	if (body)
 		body = body->scaleTo(size * GH.screenHandler().getScalingFactor(), nullptr);
 }

client/renderSDL/SDLImage.h

@@ -57,11 +57,11 @@ public:
 	void exportBitmap(const boost::filesystem::path & path, SDL_Palette * palette) const override;
 	Point dimensions() const override;
 	bool isTransparent(const Point & coords) const override;
-	std::shared_ptr<IImage> createImageReference(EImageBlitMode mode) const override;
-	std::shared_ptr<const ISharedImage> horizontalFlip() const override;
-	std::shared_ptr<const ISharedImage> verticalFlip() const override;
-	std::shared_ptr<const ISharedImage> scaleInteger(int factor, SDL_Palette * palette, EImageBlitMode blitMode) const override;
-	std::shared_ptr<const ISharedImage> scaleTo(const Point & size, SDL_Palette * palette) const override;
+	[[nodiscard]] std::shared_ptr<IImage> createImageReference(EImageBlitMode mode) const override;
+	[[nodiscard]] std::shared_ptr<const ISharedImage> horizontalFlip() const override;
+	[[nodiscard]] std::shared_ptr<const ISharedImage> verticalFlip() const override;
+	[[nodiscard]] std::shared_ptr<const ISharedImage> scaleInteger(int factor, SDL_Palette * palette, EImageBlitMode blitMode) const override;
+	[[nodiscard]] std::shared_ptr<const ISharedImage> scaleTo(const Point & size, SDL_Palette * palette) const override;
 
 	friend class SDLImageLoader;
 };

launcher/firstLaunch/firstlaunch_moc.h

@@ -55,8 +55,6 @@ class FirstLaunchView : public QWidget
 	bool checkCanInstallExtras();
 	bool checkCanInstallMod(const QString & modID);
 
-	void installMod(const QString & modID);
-
 public:
 	explicit FirstLaunchView(QWidget * parent = nullptr);
 

launcher/modManager/cmodlistview_moc.cpp

@@ -55,7 +55,7 @@ void CModListView::changeEvent(QEvent *event)
 	if(event->type() == QEvent::LanguageChange)
 	{
 		ui->retranslateUi(this);
-		modModel->reloadRepositories();
+		modModel->reloadViewModel();
 	}
 	QWidget::changeEvent(event);
 }
@@ -127,7 +127,7 @@ CModListView::CModListView(QWidget * parent)
 	ui->progressWidget->setVisible(false);
 	dlManager = nullptr;
 
-	modModel->reloadRepositories();
+	modModel->reloadViewModel();
 	if(settings["launcher"]["autoCheckRepositories"].Bool())
 		loadRepositories();
 
@@ -147,7 +147,7 @@ CModListView::CModListView(QWidget * parent)
 void CModListView::reload()
 {
 	modStateModel->reloadLocalState();
-	modModel->reloadRepositories();
+	modModel->reloadViewModel();
 }
 
 void CModListView::loadRepositories()
@@ -611,7 +611,7 @@ void CModListView::on_uninstallButton_clicked()
 		if(modStateModel->isModEnabled(modName))
 			manager->disableMod(modName);
 		manager->uninstallMod(modName);
-		modModel->reloadRepositories();
+		reload();
 	}
 	
 	checkManagerErrors();
@@ -781,7 +781,8 @@ void CModListView::installFiles(QStringList files)
 	{
 		logGlobal->info("Installing repository: started");
 		manager->setRepositoryData(accumulatedRepositoryData);
-		modModel->reloadRepositories();
+		modModel->reloadViewModel();
+		accumulatedRepositoryData.clear();
 
 		static const QString repositoryCachePath = CLauncherDirs::downloadsPath() + "/repositoryCache.json";
 		JsonUtils::jsonToFile(repositoryCachePath, modStateModel->getRepositoryData());
@@ -792,8 +793,7 @@ void CModListView::installFiles(QStringList files)
 	{
 		logGlobal->info("Installing mods: started");
 		installMods(mods);
-		modStateModel->reloadLocalState();
-		modModel->reloadRepositories();
+		reload();
 		logGlobal->info("Installing mods: ended");
 	}
 
@@ -817,8 +817,7 @@ void CModListView::installFiles(QStringList files)
 		{
 			ChroniclesExtractor ce(this, [&prog](float progress) { prog = progress; });
 			ce.installChronicles(exe);
-			modStateModel->reloadLocalState();
-			modModel->reloadRepositories();
+			reload();
 			enableModByName("chronicles");
 			return true;
 		});
@@ -835,8 +834,7 @@ void CModListView::installFiles(QStringList files)
 			ui->pushButton->setEnabled(true);
 			ui->progressWidget->setVisible(false);
 			//update
-			modStateModel->reloadLocalState();
-			modModel->reloadRepositories();
+			reload();
 		}
 		logGlobal->info("Installing chronicles: ended");
 	}
@@ -877,6 +875,8 @@ void CModListView::installMods(QStringList archives)
 		}
 	}
 
+	reload(); // FIXME: better way that won't reset selection
+
 	for(int i = 0; i < modNames.size(); i++)
 	{
 		logGlobal->info("Installing mod '%s'", modNames[i].toStdString());
@@ -884,6 +884,8 @@ void CModListView::installMods(QStringList archives)
 		manager->installMod(modNames[i], archives[i]);
 	}
 
+	reload();
+
 	if (!modsToEnable.empty())
 	{
 		manager->enableMods(modsToEnable);
@@ -1128,7 +1130,7 @@ void CModListView::deletePreset(const QString & presetName)
 void CModListView::activatePreset(const QString & presetName)
 {
 	modStateModel->activatePreset(presetName);
-	modStateModel->reloadLocalState();
+	reload();
 }
 
 void CModListView::renamePreset(const QString & oldPresetName, const QString & newPresetName)

launcher/modManager/cmodlistview_moc.ui

@@ -322,6 +322,9 @@ li.checked::marker { content: "\2612"; }
         <property name="value">
          <number>0</number>
         </property>
+        <property name="alignment">
+         <set>Qt::AlignCenter</set>
+        </property>
         <property name="textVisible">
          <bool>true</bool>
         </property>

launcher/modManager/modstatecontroller.cpp

@@ -192,9 +192,6 @@ bool ModStateController::doInstallMod(QString modname, QString archivePath)
 	if(!QFile(archivePath).exists())
 		return addError(modname, tr("Mod archive is missing"));
 
-	if(localMods.contains(modname))
-		return addError(modname, tr("Mod with such name is already installed"));
-
 	std::vector<std::string> filesToExtract;
 	QString modDirName = ::detectModArchive(archivePath, modname, filesToExtract);
 	if(!modDirName.size())
@@ -237,8 +234,6 @@ bool ModStateController::doInstallMod(QString modname, QString archivePath)
 	QString upperLevel = modDirName.section('/', 0, 0);
 	if(upperLevel != modDirName)
 		removeModDir(destDir + upperLevel);
-	
-	modList->reloadLocalState();
 
 	return true;
 }
@@ -256,8 +251,6 @@ bool ModStateController::doUninstallMod(QString modname)
 	if(!removeModDir(modDir))
 		return addError(modname, tr("Mod is located in a protected directory, please remove it manually:\n") + modFullDir.absolutePath());
 
-	modList->reloadLocalState();
-
 	return true;
 }
 

launcher/modManager/modstatecontroller.h

@@ -27,8 +27,6 @@ class ModStateController : public QObject, public boost::noncopyable
 	bool doInstallMod(QString mod, QString archivePath);
 	bool doUninstallMod(QString mod);
 
-	QVariantMap localMods;
-
 	QStringList recentErrors;
 	bool addError(QString modname, QString message);
 	bool removeModDir(QString mod);

launcher/modManager/modstateitemmodel_moc.cpp

@@ -195,7 +195,7 @@ QVariant ModStateItemModel::headerData(int section, Qt::Orientation orientation,
 	return QVariant();
 }
 
-void ModStateItemModel::reloadRepositories()
+void ModStateItemModel::reloadViewModel()
 {
 	beginResetModel();
 	endResetModel();

launcher/modManager/modstateitemmodel_moc.h

@@ -72,7 +72,7 @@ public:
 	explicit ModStateItemModel(std::shared_ptr<ModStateModel> model, QObject * parent);
 
 	/// CModListContainer overrides
-	void reloadRepositories();
+	void reloadViewModel();
 	void modChanged(QString modID);
 
 	QVariant data(const QModelIndex & index, int role) const override;

lib/modding/ModManager.cpp

@@ -654,7 +654,7 @@ void ModManager::updatePreset(const ModDependenciesResolver & testResolver)
 	for (const auto & modID : newBrokenMods)
 	{
 		const auto & mod = getModDescription(modID);
-		if (vstd::contains(newActiveMods, mod.getTopParentID()))
+		if (mod.getTopParentID().empty() || vstd::contains(newActiveMods, mod.getTopParentID()))
 			modsPreset->setModActive(modID, false);
 	}