Merge pull request #5182 from IvanSavenko/crashfixes

[1.6.2] Crashfixes for bugs discovered via Google Play

Mods/vcmi/Content/config/english.json

@@ -222,6 +222,7 @@
 
 	"vcmi.client.errors.invalidMap" : "{Invalid map or campaign}\n\nFailed to start game! Selected map or campaign might be invalid or corrupted. Reason:\n%s",
 	"vcmi.client.errors.missingCampaigns" : "{Missing data files}\n\nCampaigns data files were not found! You may be using incomplete or corrupted Heroes 3 data files. Please reinstall game data.",
+	"vcmi.client.errors.modLoadingFailure" : "{Mod loading failure}\n\nCritical issues found when loading mods! Game may not work correctly or crash! Please update or disable following mods:\n\n",
 	"vcmi.server.errors.disconnected" : "{Network Error}\n\nConnection to game server has been lost!",
 	"vcmi.server.errors.playerLeft" : "{Player Left}\n\n%s player have disconnected from the game!", //%s -> player color
 	"vcmi.server.errors.existingProcess" : "Another VCMI server process is running. Please terminate it before starting a new game.",

client/adventureMap/CList.cpp

@@ -471,8 +471,11 @@ void CTownList::CTownItem::gesture(bool on, const Point & initialPosition, const
 	int townLowerPos = (townIndex > towns.size() - 2) ? -1 : townIndex + 1;
 
 	auto updateList = [](){
-		for (auto ki : GH.windows().findWindows<CCastleInterface>())
-			ki->townChange(); //update list
+		for (auto ci : GH.windows().findWindows<CCastleInterface>())
+		{
+			ci->townlist->updateWidget();
+			ci->townlist->select(ci->town);
+		}
 	};
 
 	std::vector<RadialMenuConfig> menuElements = {

client/lobby/CSelectionBase.cpp

@@ -74,7 +74,14 @@ int ISelectionScreenInfo::getCurrentDifficulty()
 
 PlayerInfo ISelectionScreenInfo::getPlayerInfo(PlayerColor color)
 {
-	return getMapInfo()->mapHeader->players.at(color.getNum());
+	auto mapInfo = getMapInfo();
+	if (!mapInfo)
+		throw std::runtime_error("Attempt to get player info for invalid map!");
+
+	if (!mapInfo->mapHeader)
+		throw std::runtime_error("Attempt to get player info for invalid map header!");
+
+	return mapInfo->mapHeader->players.at(color.getNum());
 }
 
 CSelectionBase::CSelectionBase(ESelectionScreen type)

client/lobby/OptionsTab.cpp

@@ -933,6 +933,9 @@ void OptionsTab::SelectedBox::showPopupWindow(const Point & cursorPosition)
 
 void OptionsTab::SelectedBox::clickReleased(const Point & cursorPosition)
 {
+	if (!SEL)
+		return;
+
 	if(SEL->screenType != ESelectionScreen::newGame)
 		return;
 	

client/renderSDL/SDLImage.cpp

@@ -404,6 +404,9 @@ std::shared_ptr<IImage> SDLImageShared::createImageReference(EImageBlitMode mode
 
 std::shared_ptr<const ISharedImage> SDLImageShared::horizontalFlip() const
 {
+	if (!surf)
+		return shared_from_this();
+
 	SDL_Surface * flipped = CSDL_Ext::horizontalFlip(surf);
 	auto ret = std::make_shared<SDLImageShared>(flipped, preScaleFactor);
 	ret->fullSize = fullSize;
@@ -416,6 +419,9 @@ std::shared_ptr<const ISharedImage> SDLImageShared::horizontalFlip() const
 
 std::shared_ptr<const ISharedImage> SDLImageShared::verticalFlip() const
 {
+	if (!surf)
+		return shared_from_this();
+
 	SDL_Surface * flipped = CSDL_Ext::verticalFlip(surf);
 	auto ret = std::make_shared<SDLImageShared>(flipped, preScaleFactor);
 	ret->fullSize = fullSize;

clientapp/EntryPoint.cpp

@@ -38,7 +38,11 @@
 #include "../lib/ExceptionsCommon.h"
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/logging/CBasicLogConfigurator.h"
+#include "../lib/modding/IdentifierStorage.h"
+#include "../lib/modding/CModHandler.h"
+#include "../lib/modding/ModDescription.h"
 #include "../lib/texts/CGeneralTextHandler.h"
+#include "../lib/texts/MetaString.h"
 #include "../lib/VCMI_Lib.h"
 #include "../lib/VCMIDirs.h"
 
@@ -71,7 +75,7 @@ static void mainLoop();
 
 static CBasicLogConfigurator *logConfig;
 
-void init()
+static void init()
 {
 	CStopWatch tmh;
 	try
@@ -92,6 +96,23 @@ void init()
 	//commandController.processCommand("convert txt", false);
 }
 
+static void checkForModLoadingFailure()
+{
+	const auto & brokenMods = VLC->identifiersHandler->getModsWithFailedRequests();
+	if (!brokenMods.empty())
+	{
+		MetaString messageText;
+		messageText.appendTextID("vcmi.client.errors.modLoadingFailure");
+
+		for (const auto & modID : brokenMods)
+		{
+			messageText.appendRawString(VLC->modh->getModInfo(modID).getName());
+			messageText.appendEOL();
+		}
+		CInfoWindow::showInfoDialog(messageText.toString(), {});
+	}
+}
+
 static void prog_version()
 {
 	printf("%s\n", GameConstants::VCMI_VERSION.c_str());
@@ -386,6 +407,7 @@ int main(int argc, char * argv[])
 
 	if(!settings["session"]["headless"].Bool())
 	{
+		checkForModLoadingFailure();
 		mainLoop();
 	}
 	else

launcher/modManager/cmodlistview_moc.cpp

@@ -1053,7 +1053,13 @@ QStringList CModListView::getUpdateableMods()
 	for(const auto & modName : modStateModel->getAllMods())
 	{
 		auto mod = modStateModel->getMod(modName);
-		if (mod.isUpdateAvailable())
+		if (!mod.isUpdateAvailable())
+			continue;
+
+		QStringList notInstalledDependencies = getModsToInstall(mod.getID());
+		QStringList unavailableDependencies = findUnavailableMods(notInstalledDependencies);
+
+		if (unavailableDependencies.empty())
 			result.push_back(modName);
 	}
 

lib/CConsoleHandler.cpp

@@ -23,6 +23,15 @@ DLL_LINKAGE CConsoleHandler * console = nullptr;
 
 VCMI_LIB_NAMESPACE_END
 
+#if defined(NDEBUG) && !defined(VCMI_ANDROID)
+#define USE_ON_TERMINATE
+#endif
+
+#if defined(NDEBUG) && defined(VCMI_WINDOWS)
+#define USE_UNHANDLED_EXCEPTION_FILTER
+#define CREATE_MEMORY_DUMP
+#endif
+
 #ifndef VCMI_WINDOWS
 	using TColor = std::string;
 	#define CONSOLE_GREEN "\x1b[1;32m"
@@ -57,59 +66,7 @@ static TColor defColor;
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-#ifdef VCMI_WINDOWS
-
-void printWinError()
-{
-	//Get error code
-	int error = GetLastError();
-	if(!error)
-	{
-		logGlobal->error("No Win error information set.");
-		return;
-	}
-	logGlobal->error("Error %d encountered:", error);
-
-	//Get error description
-	char* pTemp = nullptr;
-	FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_FROM_SYSTEM,
-		nullptr, error,  MAKELANGID( LANG_NEUTRAL, SUBLANG_DEFAULT ), (LPSTR)&pTemp, 1, nullptr);
-	logGlobal->error(pTemp);
-	LocalFree( pTemp );
-}
-
-const char* exceptionName(DWORD exc)
-{
-#define EXC_CASE(EXC)	case EXCEPTION_##EXC : return "EXCEPTION_" #EXC
-	switch (exc)
-	{
-		EXC_CASE(ACCESS_VIOLATION);
-		EXC_CASE(DATATYPE_MISALIGNMENT);
-		EXC_CASE(BREAKPOINT);
-		EXC_CASE(SINGLE_STEP);
-		EXC_CASE(ARRAY_BOUNDS_EXCEEDED);
-		EXC_CASE(FLT_DENORMAL_OPERAND);
-		EXC_CASE(FLT_DIVIDE_BY_ZERO);
-		EXC_CASE(FLT_INEXACT_RESULT);
-		EXC_CASE(FLT_INVALID_OPERATION);
-		EXC_CASE(FLT_OVERFLOW);
-		EXC_CASE(FLT_STACK_CHECK);
-		EXC_CASE(FLT_UNDERFLOW);
-		EXC_CASE(INT_DIVIDE_BY_ZERO);
-		EXC_CASE(INT_OVERFLOW);
-		EXC_CASE(PRIV_INSTRUCTION);
-		EXC_CASE(IN_PAGE_ERROR);
-		EXC_CASE(ILLEGAL_INSTRUCTION);
-		EXC_CASE(NONCONTINUABLE_EXCEPTION);
-		EXC_CASE(STACK_OVERFLOW);
-		EXC_CASE(INVALID_DISPOSITION);
-		EXC_CASE(GUARD_PAGE);
-		EXC_CASE(INVALID_HANDLE);
-	default:
-		return "UNKNOWN EXCEPTION";
-	}
-#undef EXC_CASE
-}
+#ifdef CREATE_MEMORY_DUMP
 
 static void createMemoryDump(MINIDUMP_EXCEPTION_INFORMATION * meinfo)
 {
@@ -144,6 +101,42 @@ static void createMemoryDump(MINIDUMP_EXCEPTION_INFORMATION * meinfo)
 	MessageBoxA(0, "VCMI has crashed. We are sorry. File with information about encountered problem has been created.", "VCMI Crashhandler", MB_OK | MB_ICONERROR);
 }
 
+#endif
+
+#ifdef USE_UNHANDLED_EXCEPTION_FILTER
+const char* exceptionName(DWORD exc)
+{
+#define EXC_CASE(EXC)	case EXCEPTION_##EXC : return "EXCEPTION_" #EXC
+	switch (exc)
+	{
+		EXC_CASE(ACCESS_VIOLATION);
+		EXC_CASE(DATATYPE_MISALIGNMENT);
+		EXC_CASE(BREAKPOINT);
+		EXC_CASE(SINGLE_STEP);
+		EXC_CASE(ARRAY_BOUNDS_EXCEEDED);
+		EXC_CASE(FLT_DENORMAL_OPERAND);
+		EXC_CASE(FLT_DIVIDE_BY_ZERO);
+		EXC_CASE(FLT_INEXACT_RESULT);
+		EXC_CASE(FLT_INVALID_OPERATION);
+		EXC_CASE(FLT_OVERFLOW);
+		EXC_CASE(FLT_STACK_CHECK);
+		EXC_CASE(FLT_UNDERFLOW);
+		EXC_CASE(INT_DIVIDE_BY_ZERO);
+		EXC_CASE(INT_OVERFLOW);
+		EXC_CASE(PRIV_INSTRUCTION);
+		EXC_CASE(IN_PAGE_ERROR);
+		EXC_CASE(ILLEGAL_INSTRUCTION);
+		EXC_CASE(NONCONTINUABLE_EXCEPTION);
+		EXC_CASE(STACK_OVERFLOW);
+		EXC_CASE(INVALID_DISPOSITION);
+		EXC_CASE(GUARD_PAGE);
+		EXC_CASE(INVALID_HANDLE);
+	default:
+		return "UNKNOWN EXCEPTION";
+	}
+#undef EXC_CASE
+}
+
 LONG WINAPI onUnhandledException(EXCEPTION_POINTERS* exception)
 {
 	logGlobal->error("Disaster happened.");
@@ -161,14 +154,15 @@ LONG WINAPI onUnhandledException(EXCEPTION_POINTERS* exception)
 	//exception info to be placed in the dump
 	MINIDUMP_EXCEPTION_INFORMATION meinfo = {threadId, exception, TRUE};
 
+#ifdef CREATE_MEMORY_DUMP
 	createMemoryDump(&meinfo);
+#endif
 
 	return EXCEPTION_EXECUTE_HANDLER;
 }
-
 #endif
 
-#ifdef NDEBUG
+#ifdef USE_ON_TERMINATE
 [[noreturn]] static void onTerminate()
 {
 	logGlobal->error("Disaster happened.");
@@ -198,7 +192,7 @@ LONG WINAPI onUnhandledException(EXCEPTION_POINTERS* exception)
 	stream << boost::stacktrace::stacktrace();
 	logGlobal->error("%s", stream.str());
 
-#ifdef VCMI_WINDOWS
+#ifdef CREATE_MEMORY_DUMP
 	const DWORD threadId = ::GetCurrentThreadId();
 	logGlobal->error("Thread ID: %d", threadId);
 
@@ -298,19 +292,19 @@ CConsoleHandler::CConsoleHandler():
 
 	GetConsoleScreenBufferInfo(handleErr, &csbi);
 	defErrColor = csbi.wAttributes;
-#ifdef NDEBUG
-#ifndef VCMI_ANDROID
-	SetUnhandledExceptionFilter(onUnhandledException);
-#endif
-#endif
 #else
 	defColor = "\x1b[0m";
 #endif
 
-#ifdef NDEBUG
+#ifdef USE_UNHANDLED_EXCEPTION_FILTER
+	SetUnhandledExceptionFilter(onUnhandledException);
+#endif
+
+#ifdef USE_ON_TERMINATE
 	std::set_terminate(onTerminate);
 #endif
 }
+
 CConsoleHandler::~CConsoleHandler()
 {
 	logGlobal->info("Killing console...");

lib/modding/IdentifierStorage.cpp

@@ -442,6 +442,7 @@ bool CIdentifierStorage::resolveIdentifier(const ObjectCallback & request) const
 	}
 
 	// error found. Try to generate some debug info
+	failedRequests.push_back(request);
 	showIdentifierResolutionErrorDetails(request);
 	return false;
 }
@@ -494,4 +495,15 @@ void CIdentifierStorage::debugDumpIdentifiers()
 	}
 }
 
+std::vector<std::string> CIdentifierStorage::getModsWithFailedRequests() const
+{
+	std::vector<std::string> result;
+
+	for (const auto & request : failedRequests)
+		if (!vstd::contains(result, request.localScope))
+			result.push_back(request.localScope);
+
+	return result;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/modding/IdentifierStorage.h

@@ -58,6 +58,9 @@ class DLL_LINKAGE CIdentifierStorage
 	std::multimap<std::string, ObjectData> registeredObjects;
 	mutable std::vector<ObjectCallback> scheduledRequests;
 
+	/// All non-optional requests that have failed to resolve, for debug & error reporting
+	mutable std::vector<ObjectCallback> failedRequests;
+
 	ELoadingState state = ELoadingState::LOADING;
 
 	/// Helper method that dumps all registered identifier into log file
@@ -101,6 +104,9 @@ public:
 
 	/// called at the very end of loading to check for any missing ID's
 	void finalize();
+
+	/// Returns list of all mods, from which at least one identifier has failed to resolve
+	std::vector<std::string> getModsWithFailedRequests() const;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/modding/ModManager.cpp

@@ -549,7 +549,7 @@ double ModManager::getInstalledModSizeMegabytes(const TModID & modName) const
 
 void ModManager::eraseMissingModsFromPreset()
 {
-	const TModList & installedMods = modsState->getInstalledMods();
+	const TModList & installedMods = getInstalledValidMods();
 	const TModList & rootMods = modsPreset->getActiveRootMods();
 
 	modsPreset->removeOldMods(installedMods);
@@ -572,7 +572,7 @@ void ModManager::eraseMissingModsFromPreset()
 
 void ModManager::addNewModsToPreset()
 {
-	const TModList & installedMods = modsState->getInstalledMods();
+	const TModList & installedMods = getInstalledValidMods();
 
 	for(const auto & modID : installedMods)
 	{
@@ -591,6 +591,19 @@ void ModManager::addNewModsToPreset()
 	}
 }
 
+TModList ModManager::getInstalledValidMods() const
+{
+	TModList installedMods = modsState->getInstalledMods();
+	TModList validMods = modsStorage->getAllMods();
+
+	TModList result;
+	for (const auto & modID : installedMods)
+		if (vstd::contains(validMods, modID))
+			result.push_back(modID);
+
+	return result;
+}
+
 TModList ModManager::collectDependenciesRecursive(const TModID & modID) const
 {
 	TModList result;
@@ -688,7 +701,7 @@ void ModManager::updatePreset(const ModDependenciesResolver & testResolver)
 
 	for (const auto & modID : newActiveMods)
 	{
-		assert(vstd::contains(modsState->getInstalledMods(), modID));
+		assert(vstd::contains(getInstalledValidMods(), modID));
 		modsPreset->setModActive(modID, true);
 	}
 
@@ -847,7 +860,7 @@ std::tuple<std::string, TModList> ModManager::importPreset(const JsonNode & data
 	std::string presetName = modsPreset->importPreset(data);
 
 	TModList requiredMods = modsPreset->getRootMods(presetName);
-	TModList installedMods = modsState->getInstalledMods();
+	TModList installedMods = getInstalledValidMods();
 
 	TModList missingMods;
 	for (const auto & modID : requiredMods)

lib/modding/ModManager.h

@@ -132,6 +132,7 @@ class DLL_LINKAGE ModManager : boost::noncopyable
 	void addNewModsToPreset();
 	void updatePreset(const ModDependenciesResolver & newData);
 
+	TModList getInstalledValidMods() const;
 	TModList collectDependenciesRecursive(const TModID & modID) const;
 
 	void tryEnableMod(const TModID & modList);

server/NetPacksLobbyServer.cpp

@@ -454,10 +454,19 @@ void ApplyOnServerNetPackVisitor::visitLobbyDelete(LobbyDelete & pack)
 			return;
 		}
 
-		auto file = boost::filesystem::canonical(*name);
-		boost::filesystem::remove(file);
-		if(boost::filesystem::is_empty(file.parent_path()))
-			boost::filesystem::remove(file.parent_path());
+		boost::system::error_code ec;
+		auto file = boost::filesystem::canonical(*name, ec);
+
+		if (ec)
+		{
+			logGlobal->error("Failed to delete file '%s'. Reason: %s", res.getOriginalName(), ec.message());
+		}
+		else
+		{
+			boost::filesystem::remove(file);
+			if(boost::filesystem::is_empty(file.parent_path()))
+				boost::filesystem::remove(file.parent_path());
+		}
 	}
 	else if(pack.type == LobbyDelete::EType::SAVEGAME_FOLDER)
 	{
@@ -468,8 +477,17 @@ void ApplyOnServerNetPackVisitor::visitLobbyDelete(LobbyDelete & pack)
 			logGlobal->error("Failed to find folder with name '%s'", res.getOriginalName());
 			return;
 		}
+
+		boost::system::error_code ec;
 		auto folder = boost::filesystem::canonical(*name);
-		boost::filesystem::remove_all(folder);
+		if (ec)
+		{
+			logGlobal->error("Failed to delete folder '%s'. Reason: %s", res.getOriginalName(), ec.message());
+		}
+		else
+		{
+			boost::filesystem::remove_all(folder);
+		}
 	}
 
 	LobbyUpdateState lus;