From 0aa7362adfc95681ca9fb1dd33fd0599971b79e5 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Sun, 12 May 2024 20:24:09 +0000 Subject: [PATCH 01/13] Fix possible crash on accessing faction description --- client/widgets/MiscWidgets.cpp | 2 +- client/windows/CCastleInterface.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/widgets/MiscWidgets.cpp b/client/widgets/MiscWidgets.cpp index 28503f72f..20ff09686 100644 --- a/client/widgets/MiscWidgets.cpp +++ b/client/widgets/MiscWidgets.cpp @@ -466,7 +466,7 @@ void CInteractableTownTooltip::init(const CGTownInstance * town) if(town->id == townId && town->builtBuildings.count(BuildingID::TAVERN)) LOCPLINT->showTavernWindow(town, nullptr, QueryID::NONE); } - }, [&]{ + }, [town]{ if(!town->town->faction->getDescriptionTranslated().empty()) CRClickPopup::createAndPush(town->town->faction->getDescriptionTranslated()); }); diff --git a/client/windows/CCastleInterface.cpp b/client/windows/CCastleInterface.cpp index 449f87871..2fbd723c2 100644 --- a/client/windows/CCastleInterface.cpp +++ b/client/windows/CCastleInterface.cpp @@ -1351,7 +1351,7 @@ void CCastleInterface::recreateIcons() { if(town->builtBuildings.count(BuildingID::TAVERN)) LOCPLINT->showTavernWindow(town, nullptr, QueryID::NONE); - }, [&]{ + }, [this]{ if(!town->town->faction->getDescriptionTranslated().empty()) CRClickPopup::createAndPush(town->town->faction->getDescriptionTranslated()); }); From 95d761bbb8dd054862b2a2e69ef8f37895fd1520 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 15:41:47 +0000 Subject: [PATCH 02/13] Handle corrupted H3 data - show message box instead of silent crash --- client/CMT.cpp | 32 +++++++++++++++++++++++--------- lib/CArtHandler.cpp | 9 ++++++++- lib/CMakeLists.txt | 1 + lib/ExceptionsCommon.h | 16 ++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 lib/ExceptionsCommon.h diff --git a/client/CMT.cpp b/client/CMT.cpp index 10c4ea893..be8e262d5 100644 --- a/client/CMT.cpp +++ b/client/CMT.cpp @@ -31,6 +31,7 @@ #include "../lib/CConfigHandler.h" #include "../lib/CGeneralTextHandler.h" #include "../lib/CThreadHelper.h" +#include "../lib/ExceptionsCommon.h" #include "../lib/VCMIDirs.h" #include "../lib/VCMI_Lib.h" #include "../lib/filesystem/Filesystem.h" @@ -56,6 +57,7 @@ namespace po = boost::program_options; namespace po_style = boost::program_options::command_line_style; static std::atomic headlessQuit = false; +static std::optional criticalInitializationError; #ifndef VCMI_IOS void processCommand(const std::string &message); @@ -69,9 +71,16 @@ static CBasicLogConfigurator *logConfig; void init() { CStopWatch tmh; - - loadDLLClasses(); - CGI->setFromLib(); + try + { + loadDLLClasses(); + CGI->setFromLib(); + } + catch (const DataLoadingException & e) + { + criticalInitializationError = e.what(); + return; + } logGlobal->info("Initializing VCMI_Lib: %d ms", tmh.getDiff()); @@ -322,6 +331,11 @@ int main(int argc, char * argv[]) #endif // ANDROID #endif // THREADED + if (criticalInitializationError.has_value()) + { + handleFatalError(criticalInitializationError.value(), false); + } + if(!settings["session"]["headless"].Bool()) { pomtime.getDiff(); @@ -412,7 +426,7 @@ static void mainLoop() } } -[[noreturn]] static void quitApplicationImmediately() +[[noreturn]] static void quitApplicationImmediately(int error_code) { // Perform quick exit without executing static destructors and let OS cleanup anything that we did not // We generally don't care about them and this leads to numerous issues, e.g. @@ -420,9 +434,9 @@ static void mainLoop() // Android - std::quick_exit is available only starting from API level 21 // Mingw, macOS and iOS - std::quick_exit is unavailable (at least in current version of CI) #if (defined(__ANDROID_API__) && __ANDROID_API__ < 21) || (defined(__MINGW32__)) || defined(VCMI_APPLE) - ::exit(0); + ::exit(error_code); #else - std::quick_exit(0); + std::quick_exit(error_code); #endif } @@ -476,7 +490,7 @@ static void mainLoop() } std::cout << "Ending...\n"; - quitApplicationImmediately(); + quitApplicationImmediately(0); } void handleQuit(bool ask) @@ -500,7 +514,7 @@ void handleQuit(bool ask) // proper solution would be to abort init thread (or wait for it to finish) if (!CCS->curh) { - quitApplicationImmediately(); + quitApplicationImmediately(0); } if (LOCPLINT) @@ -521,5 +535,5 @@ void handleFatalError(const std::string & message, bool terminate) if (terminate) throw std::runtime_error(message); else - exit(1); + quitApplicationImmediately(1); } diff --git a/lib/CArtHandler.cpp b/lib/CArtHandler.cpp index c85d67918..9cdd430e9 100644 --- a/lib/CArtHandler.cpp +++ b/lib/CArtHandler.cpp @@ -12,6 +12,7 @@ #include "ArtifactUtils.h" #include "CGeneralTextHandler.h" +#include "ExceptionsCommon.h" #include "GameSettings.h" #include "mapObjects/MapObjects.h" #include "constants/StringConstants.h" @@ -354,7 +355,13 @@ std::vector CArtHandler::loadLegacyData() artData["slot"].Vector().emplace_back(artSlot); } } - artData["class"].String() = classes.at(parser.readString()[0]); + + std::string artClass = parser.readString(); + + if (classes.count(artClass[0])) + artData["class"].String() = classes.at(artClass[0]); + else + throw DataLoadingException("File ARTRAITS.TXT is invalid or corrupted! Please reinstall Heroes III data files"); artData["text"]["description"].String() = parser.readString(); parser.endLine(); diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 7ddfd46d5..b00c41afe 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -660,6 +660,7 @@ set(lib_MAIN_HEADERS CStack.h CStopWatch.h CTownHandler.h + ExceptionsCommon.h ExtraOptionsInfo.h FunctionList.h GameCallbackHolder.h diff --git a/lib/ExceptionsCommon.h b/lib/ExceptionsCommon.h new file mode 100644 index 000000000..b33d78106 --- /dev/null +++ b/lib/ExceptionsCommon.h @@ -0,0 +1,16 @@ +/* + * ExceptionsCommon.h, part of VCMI engine + * + * Authors: listed in file AUTHORS in main folder + * + * License: GNU General Public License v2.0 or later + * Full text of license available in license.txt file, in main folder + * + */ +#pragma once + +class DLL_LINKAGE DataLoadingException: public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; From 93da58beed55e556b01baa11ef7e5ffe13523137 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 15:42:33 +0000 Subject: [PATCH 03/13] Do not use locks on single-threaded server. Especially since this lock is global and is shared between client and server for no reason --- client/Client.cpp | 5 ++++- lib/gameState/CGameState.cpp | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/Client.cpp b/client/Client.cpp index eab9fea01..05d495004 100644 --- a/client/Client.cpp +++ b/client/Client.cpp @@ -519,7 +519,10 @@ void CClient::handlePack(CPack * pack) { apply->applyOnClBefore(this, pack); logNetwork->trace("\tMade first apply on cl: %s", typeid(*pack).name()); - gs->apply(pack); + { + boost::unique_lock lock(CGameState::mutex); + gs->apply(pack); + } logNetwork->trace("\tApplied on gs: %s", typeid(*pack).name()); apply->applyOnClAfter(this, pack); logNetwork->trace("\tMade second apply on cl: %s", typeid(*pack).name()); diff --git a/lib/gameState/CGameState.cpp b/lib/gameState/CGameState.cpp index 58163b285..aa3b67632 100644 --- a/lib/gameState/CGameState.cpp +++ b/lib/gameState/CGameState.cpp @@ -75,8 +75,6 @@ public: void applyOnGS(CGameState *gs, CPack * pack) const override { T *ptr = static_cast(pack); - - boost::unique_lock lock(CGameState::mutex); ptr->applyGs(gs); } }; From d295784f6efbacc2ad72953ee57a0f5fc79ad126 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 15:43:05 +0000 Subject: [PATCH 04/13] Shutdown server immediately without leaving hanging server in main menu --- client/CServerHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/client/CServerHandler.cpp b/client/CServerHandler.cpp index b41effc9d..fe3477fa3 100644 --- a/client/CServerHandler.cpp +++ b/client/CServerHandler.cpp @@ -421,6 +421,7 @@ void CServerHandler::sendClientDisconnecting() networkConnection->close(); networkConnection.reset(); logicConnection.reset(); + waitForServerShutdown(); } void CServerHandler::setCampaignState(std::shared_ptr newCampaign) From f8e4e41c39faf692542ae8bd620722bfdfb8da39 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 16:00:19 +0000 Subject: [PATCH 05/13] Fix possible thread race on server shutdown: - Main thread shutting down server from player request - Network thread shutting down server due to server shutting down network --- client/CServerHandler.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/client/CServerHandler.cpp b/client/CServerHandler.cpp index fe3477fa3..dea880ce3 100644 --- a/client/CServerHandler.cpp +++ b/client/CServerHandler.cpp @@ -902,6 +902,8 @@ void CServerHandler::onPacketReceived(const std::shared_ptr void CServerHandler::onDisconnected(const std::shared_ptr & connection, const std::string & errorMessage) { + boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex); + if (connection != networkConnection) { // ServerHandler already closed this connection on its own @@ -920,8 +922,6 @@ void CServerHandler::onDisconnected(const std::shared_ptr & return; } - boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex); - logNetwork->error("Lost connection to server! Connection has been closed"); if(client) @@ -956,7 +956,6 @@ void CServerHandler::waitForServerShutdown() } else { - boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex); if (getState() == EClientState::CONNECTING) { showServerError(CGI->generaltexth->translate("vcmi.server.errors.existingProcess")); From 62b564650d305d06102e5f2e4469b71c8ed7be9e Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 16:19:14 +0000 Subject: [PATCH 06/13] Fix possibly hanging pointer to deleted CGObjectInstance Was stored by CRemoveObjectOperation and accessed on destruction even though it has been removed by the game --- lib/rmg/CMapGenerator.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/rmg/CMapGenerator.cpp b/lib/rmg/CMapGenerator.cpp index fddf69f8e..34ef5fe7d 100644 --- a/lib/rmg/CMapGenerator.cpp +++ b/lib/rmg/CMapGenerator.cpp @@ -126,6 +126,9 @@ std::unique_ptr CMapGenerator::generate() fillZones(); //updated guarded tiles will be calculated in CGameState::initMapObjects() map->getZones().clear(); + + // undo manager keeps pointers to object that might be removed during gameplay. Remove them to prevent any hanging pointer after gameplay + map->getEditManager()->getUndoManager().clearAll(); } catch (rmgException &e) { From 2ddc6a145918e24d5c830cbe5d95ded6d91a0a5d Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 16:59:41 +0000 Subject: [PATCH 07/13] Android build ID bump --- android/vcmi-app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/vcmi-app/build.gradle b/android/vcmi-app/build.gradle index 4960a1551..c8bb991f0 100644 --- a/android/vcmi-app/build.gradle +++ b/android/vcmi-app/build.gradle @@ -10,7 +10,7 @@ android { applicationId "is.xyz.vcmi" minSdk 19 targetSdk 33 - versionCode 1510 + versionCode 1511 versionName "1.5.1" setProperty("archivesBaseName", "vcmi") } From 4787b9eded0a1074725589b6554d198aa799812e Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 17:25:20 +0000 Subject: [PATCH 08/13] Block movement if first node in path needs more move points that we have --- client/adventureMap/AdventureMapShortcuts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/adventureMap/AdventureMapShortcuts.cpp b/client/adventureMap/AdventureMapShortcuts.cpp index 1d4d54e17..4fa492004 100644 --- a/client/adventureMap/AdventureMapShortcuts.cpp +++ b/client/adventureMap/AdventureMapShortcuts.cpp @@ -456,7 +456,7 @@ bool AdventureMapShortcuts::optionHeroSelected() bool AdventureMapShortcuts::optionHeroCanMove() { const auto * hero = LOCPLINT->localState->getCurrentHero(); - return optionInMapView() && hero && hero->movementPointsRemaining() != 0 && LOCPLINT->localState->hasPath(hero); + return optionInMapView() && hero && LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0; } bool AdventureMapShortcuts::optionHasNextHero() From c786354af32dbd3e81a1d9ea169cf29c69968764 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 13 May 2024 21:03:06 +0000 Subject: [PATCH 09/13] Add better crash message for zero-length battlefield list --- lib/gameState/CGameState.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/gameState/CGameState.cpp b/lib/gameState/CGameState.cpp index aa3b67632..69427e7fd 100644 --- a/lib/gameState/CGameState.cpp +++ b/lib/gameState/CGameState.cpp @@ -1099,6 +1099,9 @@ BattleField CGameState::battleGetBattlefieldType(int3 tile, CRandomGenerator & r if(map->isCoastalTile(tile)) //coastal tile is always ground return BattleField(*VLC->identifiers()->getIdentifier("core", "battlefield.sand_shore")); + if (t.terType->battleFields.empty()) + throw std::runtime_error("Failed to find battlefield for terrain " + t.terType->getJsonKey()); + return BattleField(*RandomGeneratorUtil::nextItem(t.terType->battleFields, rand)); } From c92a5bbbab8c632c88c809179db24b19025fc158 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 14 May 2024 09:44:40 +0000 Subject: [PATCH 10/13] Attempt to track crashes with unclear cause --- client/widgets/Images.cpp | 8 ++++-- lib/MetaString.cpp | 42 ++++++++++++++++---------------- lib/networkPacks/NetPacksLib.cpp | 16 ++++++------ 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/client/widgets/Images.cpp b/client/widgets/Images.cpp index f4ce12ce3..6699476da 100644 --- a/client/widgets/Images.cpp +++ b/client/widgets/Images.cpp @@ -334,8 +334,12 @@ CShowableAnim::CShowableAnim(int x, int y, const AnimationPath & name, ui8 Flags anim->loadGroup(group); last = anim->size(group); - pos.w = anim->getImage(0, group)->width(); - pos.h = anim->getImage(0, group)->height(); + auto image = anim->getImage(0, group); + if (!image) + throw std::runtime_error("Failed to load group " + std::to_string(group) + " of animation file " + name.getOriginalName()); + + pos.w = image->width(); + pos.h = image->height(); pos.x+= x; pos.y+= y; diff --git a/lib/MetaString.cpp b/lib/MetaString.cpp index 92d84e851..db2e015dd 100644 --- a/lib/MetaString.cpp +++ b/lib/MetaString.cpp @@ -144,33 +144,33 @@ DLL_LINKAGE std::string MetaString::toString() const switch(elem) { case EMessage::APPEND_RAW_STRING: - dst += exactStrings[exSt++]; + dst += exactStrings.at(exSt++); break; case EMessage::APPEND_LOCAL_STRING: - dst += getLocalString(localStrings[loSt++]); + dst += getLocalString(localStrings.at(loSt++)); break; case EMessage::APPEND_TEXTID_STRING: - dst += VLC->generaltexth->translate(stringsTextID[textID++]); + dst += VLC->generaltexth->translate(stringsTextID.at(textID++)); break; case EMessage::APPEND_NUMBER: - dst += std::to_string(numbers[nums++]); + dst += std::to_string(numbers.at(nums++)); break; case EMessage::REPLACE_RAW_STRING: - boost::replace_first(dst, "%s", exactStrings[exSt++]); + boost::replace_first(dst, "%s", exactStrings.at(exSt++)); break; case EMessage::REPLACE_LOCAL_STRING: - boost::replace_first(dst, "%s", getLocalString(localStrings[loSt++])); + boost::replace_first(dst, "%s", getLocalString(localStrings.at(loSt++))); break; case EMessage::REPLACE_TEXTID_STRING: - boost::replace_first(dst, "%s", VLC->generaltexth->translate(stringsTextID[textID++])); + boost::replace_first(dst, "%s", VLC->generaltexth->translate(stringsTextID.at(textID++))); break; case EMessage::REPLACE_NUMBER: - boost::replace_first(dst, "%d", std::to_string(numbers[nums++])); + boost::replace_first(dst, "%d", std::to_string(numbers.at(nums++))); break; case EMessage::REPLACE_POSITIVE_NUMBER: if (dst.find("%+d") != std::string::npos) { - int64_t value = numbers[nums]; + int64_t value = numbers.at(nums); if (value > 0) boost::replace_first(dst, "%+d", '+' + std::to_string(value)); else @@ -179,7 +179,7 @@ DLL_LINKAGE std::string MetaString::toString() const nums++; } else - boost::replace_first(dst, "%d", std::to_string(numbers[nums++])); + boost::replace_first(dst, "%d", std::to_string(numbers.at(nums++))); break; default: logGlobal->error("MetaString processing error! Received message of type %d", static_cast(elem)); @@ -199,41 +199,41 @@ DLL_LINKAGE std::string MetaString::buildList() const std::string lista; for(int i = 0; i < message.size(); ++i) { - if(i > 0 && (message[i] == EMessage::APPEND_RAW_STRING || message[i] == EMessage::APPEND_LOCAL_STRING)) + if(i > 0 && (message.at(i) == EMessage::APPEND_RAW_STRING || message.at(i) == EMessage::APPEND_LOCAL_STRING)) { if(exSt == exactStrings.size() - 1) lista += VLC->generaltexth->allTexts[141]; //" and " else lista += ", "; } - switch(message[i]) + switch(message.at(i)) { case EMessage::APPEND_RAW_STRING: - lista += exactStrings[exSt++]; + lista += exactStrings.at(exSt++); break; case EMessage::APPEND_LOCAL_STRING: - lista += getLocalString(localStrings[loSt++]); + lista += getLocalString(localStrings.at(loSt++)); break; case EMessage::APPEND_TEXTID_STRING: - lista += VLC->generaltexth->translate(stringsTextID[textID++]); + lista += VLC->generaltexth->translate(stringsTextID.at(textID++)); break; case EMessage::APPEND_NUMBER: - lista += std::to_string(numbers[nums++]); + lista += std::to_string(numbers.at(nums++)); break; case EMessage::REPLACE_RAW_STRING: - lista.replace(lista.find("%s"), 2, exactStrings[exSt++]); + lista.replace(lista.find("%s"), 2, exactStrings.at(exSt++)); break; case EMessage::REPLACE_LOCAL_STRING: - lista.replace(lista.find("%s"), 2, getLocalString(localStrings[loSt++])); + lista.replace(lista.find("%s"), 2, getLocalString(localStrings.at(loSt++))); break; case EMessage::REPLACE_TEXTID_STRING: - lista.replace(lista.find("%s"), 2, VLC->generaltexth->translate(stringsTextID[textID++])); + lista.replace(lista.find("%s"), 2, VLC->generaltexth->translate(stringsTextID.at(textID++))); break; case EMessage::REPLACE_NUMBER: - lista.replace(lista.find("%d"), 2, std::to_string(numbers[nums++])); + lista.replace(lista.find("%d"), 2, std::to_string(numbers.at(nums++))); break; default: - logGlobal->error("MetaString processing error! Received message of type %d", int(message[i])); + logGlobal->error("MetaString processing error! Received message of type %d", int(message.at(i))); } } return lista; diff --git a/lib/networkPacks/NetPacksLib.cpp b/lib/networkPacks/NetPacksLib.cpp index 28d8779d7..ead5778d2 100644 --- a/lib/networkPacks/NetPacksLib.cpp +++ b/lib/networkPacks/NetPacksLib.cpp @@ -1579,7 +1579,7 @@ void ChangeStackCount::applyGs(CGameState * gs) { auto * srcObj = gs->getArmyInstance(army); if(!srcObj) - logNetwork->error("[CRITICAL] ChangeStackCount: invalid army object %d, possible game state corruption.", army.getNum()); + throw std::runtime_error("ChangeStackCount: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption."); if(absoluteValue) srcObj->setStackCount(slot, count); @@ -1591,7 +1591,7 @@ void SetStackType::applyGs(CGameState * gs) { auto * srcObj = gs->getArmyInstance(army); if(!srcObj) - logNetwork->error("[CRITICAL] SetStackType: invalid army object %d, possible game state corruption.", army.getNum()); + throw std::runtime_error("SetStackType: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption."); srcObj->setStackType(slot, type); } @@ -1600,7 +1600,7 @@ void EraseStack::applyGs(CGameState * gs) { auto * srcObj = gs->getArmyInstance(army); if(!srcObj) - logNetwork->error("[CRITICAL] EraseStack: invalid army object %d, possible game state corruption.", army.getNum()); + throw std::runtime_error("EraseStack: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption."); srcObj->eraseStack(slot); } @@ -1609,11 +1609,11 @@ void SwapStacks::applyGs(CGameState * gs) { auto * srcObj = gs->getArmyInstance(srcArmy); if(!srcObj) - logNetwork->error("[CRITICAL] SwapStacks: invalid army object %d, possible game state corruption.", srcArmy.getNum()); + throw std::runtime_error("SwapStacks: invalid army object " + std::to_string(srcArmy.getNum()) + ", possible game state corruption."); auto * dstObj = gs->getArmyInstance(dstArmy); if(!dstObj) - logNetwork->error("[CRITICAL] SwapStacks: invalid army object %d, possible game state corruption.", dstArmy.getNum()); + throw std::runtime_error("SwapStacks: invalid army object " + std::to_string(dstArmy.getNum()) + ", possible game state corruption."); CStackInstance * s1 = srcObj->detachStack(srcSlot); CStackInstance * s2 = dstObj->detachStack(dstSlot); @@ -1627,18 +1627,18 @@ void InsertNewStack::applyGs(CGameState *gs) if(auto * obj = gs->getArmyInstance(army)) obj->putStack(slot, new CStackInstance(type, count)); else - logNetwork->error("[CRITICAL] InsertNewStack: invalid army object %d, possible game state corruption.", army.getNum()); + throw std::runtime_error("InsertNewStack: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption."); } void RebalanceStacks::applyGs(CGameState * gs) { auto * srcObj = gs->getArmyInstance(srcArmy); if(!srcObj) - logNetwork->error("[CRITICAL] RebalanceStacks: invalid army object %d, possible game state corruption.", srcArmy.getNum()); + throw std::runtime_error("RebalanceStacks: invalid army object " + std::to_string(srcArmy.getNum()) + ", possible game state corruption."); auto * dstObj = gs->getArmyInstance(dstArmy); if(!dstObj) - logNetwork->error("[CRITICAL] RebalanceStacks: invalid army object %d, possible game state corruption.", dstArmy.getNum()); + throw std::runtime_error("RebalanceStacks: invalid army object " + std::to_string(dstArmy.getNum()) + ", possible game state corruption."); StackLocation src(srcObj, srcSlot); StackLocation dst(dstObj, dstSlot); From 8807e05ee2f73d173f4441936da2d2c3aea863c8 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 14 May 2024 09:54:14 +0000 Subject: [PATCH 11/13] Update changelog --- ChangeLog.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 3f2d41954..5dd3b17e1 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,3 +1,14 @@ +# 1.5.0 -> 1.5.1 + +### Stability +* Fixed possible crash on accessing faction description +* Fixed possible thread race on leaving to main menu +* Game will now show error message instead of silent crash on corrupted H3 data +* Fixed possible crash on double-deletion of quest artifacts placed by RMG + +### Interface +* Fixed possible freeze on attempt to move hero when hero has non-zero movement points but not enough to reach first tile in path + # 1.4.5 -> 1.5.0 ### General From 22d51bd4737d9d4e5eb605c9b9e8cc23f5493296 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 14 May 2024 12:32:22 +0000 Subject: [PATCH 12/13] Fix freeze on closing game window while in game --- client/CServerHandler.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/CServerHandler.cpp b/client/CServerHandler.cpp index dea880ce3..330fb8b78 100644 --- a/client/CServerHandler.cpp +++ b/client/CServerHandler.cpp @@ -119,7 +119,10 @@ CServerHandler::~CServerHandler() if (serverRunner) serverRunner->wait(); serverRunner.reset(); - threadNetwork.join(); + { + auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex); + threadNetwork.join(); + } } catch (const std::runtime_error & e) { From 00728ded88b762d7457051bbcff5152d9388fdd5 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 14 May 2024 12:48:19 +0000 Subject: [PATCH 13/13] Report more information on exception --- lib/network/NetworkConnection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/network/NetworkConnection.cpp b/lib/network/NetworkConnection.cpp index a034fdd7b..56a1c57c6 100644 --- a/lib/network/NetworkConnection.cpp +++ b/lib/network/NetworkConnection.cpp @@ -92,7 +92,7 @@ void NetworkConnection::onPacketReceived(const boost::system::error_code & ec, u if (readBuffer.size() < expectedPacketSize) { - throw std::runtime_error("Failed to read packet!"); + throw std::runtime_error("Failed to read packet! " + std::to_string(readBuffer.size()) + " bytes read, but " + std::to_string(expectedPacketSize) + " bytes expected!"); } std::vector message(expectedPacketSize);