From 67b7c39761066c886565264b12948c2267a0a7ce Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 10 Aug 2023 21:18:36 +0300 Subject: [PATCH 1/9] Fix possible nullptr dereference --- AI/Nullkiller/Behaviors/DefenceBehavior.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AI/Nullkiller/Behaviors/DefenceBehavior.cpp b/AI/Nullkiller/Behaviors/DefenceBehavior.cpp index 3e1ee23f2..d9bdf3904 100644 --- a/AI/Nullkiller/Behaviors/DefenceBehavior.cpp +++ b/AI/Nullkiller/Behaviors/DefenceBehavior.cpp @@ -189,7 +189,7 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta town->getNameTranslated(), treat.danger, std::to_string(treat.turn), - treat.hero->getNameTranslated()); + treat.hero ? treat.hero->getNameTranslated() : std::string("")); handleCounterAttack(town, treat, treatNode.maximumDanger, tasks); From 942f8bbf058c59aab091455771f0dfbbc3420d98 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 10 Aug 2023 21:18:52 +0300 Subject: [PATCH 2/9] Fix possible access to non-existing building --- lib/mapObjects/CGTownInstance.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index 4f088b414..10fd30409 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -996,6 +996,12 @@ CBuilding::TRequired CGTownInstance::genBuildingRequirements(const BuildingID & std::function dependTest = [&](const BuildingID & id) -> CBuilding::TRequired::Variant { + if (town->buildings.count(id) == 0) + { + logMod->error("Invalid building ID %d in building dependencies!", id.getNum()); + return CBuilding::TRequired::OperatorAll(); + } + const CBuilding * build = town->buildings.at(id); CBuilding::TRequired::OperatorAll requirements; From 631e93e846f108195fbe5fd9dae26e4cd0f7ff2b Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 10 Aug 2023 21:19:12 +0300 Subject: [PATCH 3/9] Fix missing lock of player interface --- client/gui/CGuiHandler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/gui/CGuiHandler.cpp b/client/gui/CGuiHandler.cpp index fdccf1517..27c97895a 100644 --- a/client/gui/CGuiHandler.cpp +++ b/client/gui/CGuiHandler.cpp @@ -88,6 +88,8 @@ void CGuiHandler::handleEvents() void CGuiHandler::fakeMouseMove() { dispatchMainThread([](){ + assert(CPlayerInterface::pim); + boost::unique_lock lock(*CPlayerInterface::pim); GH.events().dispatchMouseMoved(Point(0, 0), GH.getCursorPosition()); }); } From 9d7f46f98507a715fb1c631e9dffb247c3195135 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 10 Aug 2023 21:21:37 +0300 Subject: [PATCH 4/9] Fix crash on right-clicking player flags in RMG UI --- client/lobby/RandomMapTab.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/lobby/RandomMapTab.cpp b/client/lobby/RandomMapTab.cpp index dfb0157cb..2a0ca914f 100644 --- a/client/lobby/RandomMapTab.cpp +++ b/client/lobby/RandomMapTab.cpp @@ -147,7 +147,6 @@ void RandomMapTab::updateMapInfoByHost() mapInfo->mapHeader->twoLevel = mapGenOptions->getHasTwoLevels(); // Generate player information - mapInfo->mapHeader->players.clear(); int playersToGen = PlayerColor::PLAYER_LIMIT_I; if(mapGenOptions->getPlayerCount() != CMapGenOptions::RANDOM_SIZE) { @@ -157,10 +156,15 @@ void RandomMapTab::updateMapInfoByHost() playersToGen = mapGenOptions->getPlayerCount(); } - mapInfo->mapHeader->howManyTeams = playersToGen; std::set occupiedTeams; + for(int i = 0; i < PlayerColor::PLAYER_LIMIT_I; ++i) + { + mapInfo->mapHeader->players[i].canComputerPlay = false; + mapInfo->mapHeader->players[i].canHumanPlay = false; + } + for(int i = 0; i < playersToGen; ++i) { PlayerInfo player; @@ -179,7 +183,7 @@ void RandomMapTab::updateMapInfoByHost() occupiedTeams.insert(team); player.hasMainTown = true; player.generateHeroAtMainTown = true; - mapInfo->mapHeader->players.push_back(player); + mapInfo->mapHeader->players[i] = player; } for(auto & player : mapInfo->mapHeader->players) { From ad2bd897d04f71411a0bdb20f4d3176efc460bfb Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 10 Aug 2023 21:23:18 +0300 Subject: [PATCH 5/9] Replace boost::format with MetaString Looks like some of H3 localizations don't have string replacements for building / war machine name, resulting in exceptions on formatting attempt. MetaString handles this case silently. --- client/windows/CCastleInterface.cpp | 48 ++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/client/windows/CCastleInterface.cpp b/client/windows/CCastleInterface.cpp index c5a1327bf..2f6e20f58 100644 --- a/client/windows/CCastleInterface.cpp +++ b/client/windows/CCastleInterface.cpp @@ -1451,7 +1451,11 @@ CBuildWindow::CBuildWindow(const CGTownInstance *Town, const CBuilding * Buildin auto statusbarBackground = std::make_shared(background->getSurface(), Rect(8, pos.h - 26, pos.w - 16, 19), 8, pos.h - 26); statusbar = CGStatusBar::create(statusbarBackground); - name = std::make_shared(197, 30, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, boost::str(boost::format(CGI->generaltexth->hcommands[7]) % building->getNameTranslated())); + MetaString nameString; + nameString.appendTextID("core.hallinfo.7"); + nameString.replaceTextID(building->getNameTextID()); + + name = std::make_shared(197, 30, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, nameString.toString()); description = std::make_shared(building->getDescriptionTranslated(), Rect(33, 135, 329, 67), 0, FONT_MEDIUM, ETextAlignment::CENTER); stateText = std::make_shared(getTextForState(state), Rect(33, 216, 329, 67), 0, FONT_SMALL, ETextAlignment::CENTER); @@ -1468,14 +1472,20 @@ CBuildWindow::CBuildWindow(const CGTownInstance *Town, const CBuilding * Buildin if(!rightClick) { //normal window - std::string tooltipYes = boost::str(boost::format(CGI->generaltexth->allTexts[595]) % building->getNameTranslated()); - std::string tooltipNo = boost::str(boost::format(CGI->generaltexth->allTexts[596]) % building->getNameTranslated()); - buy = std::make_shared(Point(45, 446), "IBUY30", CButton::tooltip(tooltipYes), [&](){ buyFunc(); }, EShortcut::GLOBAL_ACCEPT); + MetaString tooltipYes; + tooltipYes.appendTextID("core.genrltxt.595"); + tooltipYes.replaceTextID(building->getNameTextID()); + + MetaString tooltipNo; + tooltipNo.appendTextID("core.genrltxt.596"); + tooltipNo.replaceTextID(building->getNameTextID()); + + buy = std::make_shared(Point(45, 446), "IBUY30", CButton::tooltip(tooltipYes.toString()), [&](){ buyFunc(); }, EShortcut::GLOBAL_ACCEPT); buy->setBorderColor(Colors::METALLIC_GOLD); buy->block(state!=7 || LOCPLINT->playerID != town->tempOwner); - cancel = std::make_shared(Point(290, 445), "ICANCEL", CButton::tooltip(tooltipNo), [&](){ close();}, EShortcut::GLOBAL_CANCEL); + cancel = std::make_shared(Point(290, 445), "ICANCEL", CButton::tooltip(tooltipNo.toString()), [&](){ close();}, EShortcut::GLOBAL_CANCEL); cancel->setBorderColor(Colors::METALLIC_GOLD); } } @@ -1841,17 +1851,25 @@ CBlacksmithDialog::CBlacksmithDialog(bool possible, CreatureID creMachineID, Art anim = std::make_shared(64, 50, creature->animDefName); anim->clipRect(113,125,200,150); - title = std::make_shared(165, 28, FONT_BIG, ETextAlignment::CENTER, Colors::YELLOW, - boost::str(boost::format(CGI->generaltexth->allTexts[274]) % creature->getNameSingularTranslated())); + MetaString titleString; + titleString.appendTextID("core.genrltxt.274"); + titleString.replaceTextID(creature->getNameSingularTextID()); + + MetaString buyText; + buyText.appendTextID("core.genrltxt.595"); + buyText.replaceTextID(creature->getNameSingularTextID()); + + MetaString cancelText; + cancelText.appendTextID("core.genrltxt.596"); + cancelText.replaceTextID(creature->getNameSingularTextID()); + + std::string costString = std::to_string(aid.toArtifact(CGI->artifacts())->getPrice()); + + title = std::make_shared(165, 28, FONT_BIG, ETextAlignment::CENTER, Colors::YELLOW, titleString.toString()); costText = std::make_shared(165, 218, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->jktexts[43]); - costValue = std::make_shared(165, 292, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, - std::to_string(aid.toArtifact(CGI->artifacts())->getPrice())); - - std::string text = boost::str(boost::format(CGI->generaltexth->allTexts[595]) % creature->getNameSingularTranslated()); - buy = std::make_shared(Point(42, 312), "IBUY30.DEF", CButton::tooltip(text), [&](){ close(); }, EShortcut::GLOBAL_ACCEPT); - - text = boost::str(boost::format(CGI->generaltexth->allTexts[596]) % creature->getNameSingularTranslated()); - cancel = std::make_shared(Point(224, 312), "ICANCEL.DEF", CButton::tooltip(text), [&](){ close(); }, EShortcut::GLOBAL_CANCEL); + costValue = std::make_shared(165, 292, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, costString); + buy = std::make_shared(Point(42, 312), "IBUY30.DEF", CButton::tooltip(buyText.toString()), [&](){ close(); }, EShortcut::GLOBAL_ACCEPT); + cancel = std::make_shared(Point(224, 312), "ICANCEL.DEF", CButton::tooltip(cancelText.toString()), [&](){ close(); }, EShortcut::GLOBAL_CANCEL); if(possible) buy->addCallback([=](){ LOCPLINT->cb->buyArtifact(LOCPLINT->cb->getHero(hid),aid); }); From 4b307dc0e4d776fd3941ec92a2fd1ef989c738e4 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Fri, 11 Aug 2023 18:50:00 +0300 Subject: [PATCH 6/9] More locks to avoid data races on server --- server/CGameHandler.cpp | 2 ++ server/NetPacksServer.cpp | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 3cb0f5655..3599c399a 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -6464,6 +6464,8 @@ void CGameHandler::runBattle() bool CGameHandler::makeAutomaticAction(const CStack *stack, BattleAction &ba) { + boost::unique_lock lock(battleActionMutex); + BattleSetActiveStack bsa; bsa.stack = stack->unitId(); bsa.askPlayerInterface = false; diff --git a/server/NetPacksServer.cpp b/server/NetPacksServer.cpp index f2f37e4a4..7660bc073 100644 --- a/server/NetPacksServer.cpp +++ b/server/NetPacksServer.cpp @@ -25,6 +25,8 @@ #include "../lib/spells/ISpellMechanics.h" #include "../lib/serializer/Cast.h" +extern boost::recursive_mutex battleActionMutex; + void ApplyGhNetPackVisitor::visitSaveGame(SaveGame & pack) { gh.save(pack.fname); @@ -280,6 +282,8 @@ void ApplyGhNetPackVisitor::visitQueryReply(QueryReply & pack) void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack) { + boost::unique_lock lock(battleActionMutex); + const BattleInfo * b = gs.curB; if(!b) gh.throwAndComplain(&pack, "Can not make action - there is no battle ongoing!"); @@ -307,6 +311,8 @@ void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack) void ApplyGhNetPackVisitor::visitMakeCustomAction(MakeCustomAction & pack) { + boost::unique_lock lock(battleActionMutex); + const BattleInfo * b = gs.curB; if(!b) gh.throwNotAllowedAction(&pack); From 119d51d1efe6e0a92e3e368c81cb6e5df5c76a96 Mon Sep 17 00:00:00 2001 From: SoundSSGood <87084363+SoundSSGood@users.noreply.github.com> Date: Fri, 11 Aug 2023 14:19:06 +0300 Subject: [PATCH 7/9] altar sacrifice all fix --- client/windows/CTradeWindow.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/client/windows/CTradeWindow.cpp b/client/windows/CTradeWindow.cpp index 6787308fd..7787ec4de 100644 --- a/client/windows/CTradeWindow.cpp +++ b/client/windows/CTradeWindow.cpp @@ -1262,11 +1262,14 @@ void CAltarWindow::SacrificeAll() } else { - for(const auto & aw : arts->visibleArtSet.artifactsWorn) + std::vector> artsForMove; + for(const auto& slotInfo : arts->visibleArtSet.artifactsWorn) { - if(!aw.second.locked) - moveArtToAltar(nullptr, aw.second.artifact); + if(!slotInfo.second.locked && slotInfo.second.artifact->artType->isTradable()) + artsForMove.push_back(slotInfo.second.artifact); } + for(auto artInst : artsForMove) + moveArtToAltar(nullptr, artInst); arts->updateWornSlots(); SacrificeBackpack(); } From 20e10cd5abfb2c6fc66554098680af07e05ea26f Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Wed, 9 Aug 2023 00:46:55 +0300 Subject: [PATCH 8/9] Fix potential access to empty std function on hero vs hero combat --- client/battle/BattleInterfaceClasses.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/battle/BattleInterfaceClasses.cpp b/client/battle/BattleInterfaceClasses.cpp index a6289d07f..7f2f0647d 100644 --- a/client/battle/BattleInterfaceClasses.cpp +++ b/client/battle/BattleInterfaceClasses.cpp @@ -636,7 +636,9 @@ void BattleResultWindow::show(Canvas & to) void BattleResultWindow::buttonPressed(int button) { - resultCallback(button); + if (resultCallback) + resultCallback(button); + CPlayerInterface &intTmp = owner; //copy reference because "this" will be destructed soon close(); From 4ebb49074082abc93921fd273a0727d43f3bcdfd Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Fri, 11 Aug 2023 18:53:13 +0300 Subject: [PATCH 9/9] Android build version bump for hotfix --- 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 0a00e9de0..5e67599a8 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 31 - versionCode 1304 + versionCode 1305 versionName "1.3.0" setProperty("archivesBaseName", "vcmi") }