From 099a16815875472d2dde1cdd054d66dcd7e463ff Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:52:48 +0300 Subject: [PATCH 01/11] Fix crash on AI capturing artifacts in battle while covered by FoW --- client/NetPacksClient.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/NetPacksClient.cpp b/client/NetPacksClient.cpp index 4a8547a8d..29a8558f9 100644 --- a/client/NetPacksClient.cpp +++ b/client/NetPacksClient.cpp @@ -828,7 +828,7 @@ void ApplyClientNetPackVisitor::visitBattleResultsApplied(BattleResultsApplied & { if(!pack.learnedSpells.spells.empty()) { - const auto hero = GAME->interface()->cb->getHero(pack.learnedSpells.hid); + const auto * hero = cl.gameInfo().getHero(pack.learnedSpells.hid); assert(hero); callInterfaceIfPresent(cl, pack.victor, &CGameInterface::showInfoDialog, EInfoWindowMode::MODAL, UIHelper::getEagleEyeInfoWindowText(*hero, pack.learnedSpells.spells), UIHelper::getSpellsComponents(pack.learnedSpells.spells), soundBase::soundID(0)); @@ -836,7 +836,7 @@ void ApplyClientNetPackVisitor::visitBattleResultsApplied(BattleResultsApplied & if(!pack.movingArtifacts.empty()) { - const auto artSet = GAME->interface()->cb->getArtSet(ArtifactLocation(pack.movingArtifacts.front().dstArtHolder)); + const auto * artSet = cl.gameState().getArtSet(ArtifactLocation(pack.movingArtifacts.front().dstArtHolder)); assert(artSet); std::vector artComponents; for(const auto & artPack : pack.movingArtifacts) From 1744ebffdc8cd75ae1078ecc16efda7125ddbf46 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:53:23 +0300 Subject: [PATCH 02/11] Fix quick recruitment failing when there are no free slots in army --- client/windows/QuickRecruitmentWindow.cpp | 41 ++++++++++++++--------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/client/windows/QuickRecruitmentWindow.cpp b/client/windows/QuickRecruitmentWindow.cpp index b4ab05f16..72ce85c99 100644 --- a/client/windows/QuickRecruitmentWindow.cpp +++ b/client/windows/QuickRecruitmentWindow.cpp @@ -103,26 +103,35 @@ void QuickRecruitmentWindow::maxAllCards(std::vectorgetUpperArmy()->getFreeSlots().size(); + for(auto selected : boost::adaptors::reverse(cards)) { - if(selected->slider->getValue()) + if(selected->slider->getValue() == 0) + continue; + + int level = 0; + int i = 0; + for(auto c : town->getTown()->creatures) { - int level = 0; - int i = 0; - for(auto c : town->getTown()->creatures) - { - for(auto c2 : c) - if(c2 == selected->creatureOnTheCard->getId()) - level = i; - i++; - } - auto onRecruit = [this, level](CreatureID id, int count){ GAME->interface()->cb->recruitCreatures(town, town->getUpperArmy(), id, count, level); }; - CreatureID crid = selected->creatureOnTheCard->getId(); - SlotID dstslot = town -> getSlotFor(crid); - if(!dstslot.validSlot()) - continue; - onRecruit(crid, selected->slider->getValue()); + for(auto c2 : c) + if(c2 == selected->creatureOnTheCard->getId()) + level = i; + i++; } + + CreatureID crid = selected->creatureOnTheCard->getId(); + SlotID dstslot = town->getUpperArmy()->getSlotFor(crid); + + if(town->getUpperArmy()->slotEmpty(dstslot)) + { + if(freeSlotsLeft == 0) + continue; + freeSlotsLeft -= 1; + } + + if(dstslot.validSlot()) + GAME->interface()->cb->recruitCreatures(town, town->getUpperArmy(), crid, selected->slider->getValue(), level); } close(); } From a036482a7d801f74e2b547f2503848c3f2b930ed Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:53:55 +0300 Subject: [PATCH 03/11] Fix regresssion - crash on transferring hero to next scenario --- lib/campaign/CampaignState.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/campaign/CampaignState.cpp b/lib/campaign/CampaignState.cpp index 88c7a659a..393ef16bb 100644 --- a/lib/campaign/CampaignState.cpp +++ b/lib/campaign/CampaignState.cpp @@ -316,6 +316,8 @@ JsonNode CampaignState::crossoverSerialize(CGHeroInstance * hero) const JsonNode node; JsonSerializer handler(nullptr, node); hero->serializeJsonOptions(handler); + node.setModScope(ModScope::scopeGame()); + logGlobal->info(node.toString()); return node; } From 03997a800a2b2bdb60934473edfc06eed88b5995 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:54:29 +0300 Subject: [PATCH 04/11] Add workaround for loading save when entity was moved to another mod --- lib/constants/EntityIdentifiers.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/constants/EntityIdentifiers.cpp b/lib/constants/EntityIdentifiers.cpp index 4f03c5935..17421a685 100644 --- a/lib/constants/EntityIdentifiers.cpp +++ b/lib/constants/EntityIdentifiers.cpp @@ -158,6 +158,16 @@ int32_t IdentifierBase::resolveIdentifier(const std::string & entityType, const if (rawId) return rawId.value(); + + size_t semicolon = identifier.find(':'); + + if (semicolon != std::string::npos) + { + auto rawId2 = LIBRARY->identifiers()->getIdentifier(ModScope::scopeGame(), entityType, identifier.substr(semicolon + 1)); + if (rawId2) + return rawId2.value(); + } + throw IdentifierResolutionException(identifier); } From 800ccf2651e4835c5680a39f6adcc308ceb85abf Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:54:56 +0300 Subject: [PATCH 05/11] Fix Solmyr/Yog receiving spellbook on transferring to next scenario --- lib/gameState/CGameStateCampaign.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/gameState/CGameStateCampaign.cpp b/lib/gameState/CGameStateCampaign.cpp index d89f26165..294584517 100644 --- a/lib/gameState/CGameStateCampaign.cpp +++ b/lib/gameState/CGameStateCampaign.cpp @@ -197,6 +197,10 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(vstd::RNG & randomGenerat hero.hero->eraseStack(slotID); } + // Add spell flag to ensure that hero without spellbook won't receive one as part of initHero call + for(auto & hero : campaignHeroReplacements) + hero.hero->addSpellToSpellbook(SpellID::SPELLBOOK_PRESET); + // Removing short-term bonuses for(auto & hero : campaignHeroReplacements) { From bfc7ca8c8603790e4ad1f73e4f4d8582bee904c1 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:55:46 +0300 Subject: [PATCH 06/11] Fix regression - crash on attempt to load saved game --- server/CGameHandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 1c82b7170..941b6f215 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -1606,8 +1606,8 @@ void CGameHandler::save(const std::string & filename) void CGameHandler::load(const StartInfo &info) { - logGlobal->info("Loading from %s", info.fileURI); - const auto stem = FileInfo::GetPathStem(info.fileURI); + logGlobal->info("Loading from %s", info.mapname); + const auto stem = FileInfo::GetPathStem(info.mapname); reinitScripting(); From 760eff81395446b55bf64fae4ef2f9143b57f883 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:57:28 +0300 Subject: [PATCH 07/11] Fix crash on loading town with unit that has ability propagated to army --- lib/mapping/MapFormatH3M.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/mapping/MapFormatH3M.cpp b/lib/mapping/MapFormatH3M.cpp index c9a2923b8..2b038733a 100644 --- a/lib/mapping/MapFormatH3M.cpp +++ b/lib/mapping/MapFormatH3M.cpp @@ -2474,7 +2474,10 @@ std::shared_ptr CMapLoaderH3M::readTown(const int3 & position, std::optional faction; if (objectTemplate->id == Obj::TOWN) + { faction = FactionID(objectTemplate->subid); + object->subID = objectTemplate->subid; + } bool hasName = reader->readBool(); if(hasName) From 2ad75bbde718b5770bf3de357f89447cf1f2880d Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:57:52 +0300 Subject: [PATCH 08/11] Add check for victory condition on assembling artifact --- server/CGameHandler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 941b6f215..f6ef0d499 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -2947,6 +2947,8 @@ bool CGameHandler::assembleArtifacts(ObjectInstanceID heroID, ArtifactPosition a sendAndApply(da); } + checkVictoryLossConditionsForPlayer(hero->getOwner()); + return true; } From f773b87cd5cb255801c2959de2f449363b6ca4a2 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Thu, 31 Jul 2025 21:58:27 +0300 Subject: [PATCH 09/11] Add debug logging for mods with invalid town building config --- lib/entities/faction/CTownHandler.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/entities/faction/CTownHandler.cpp b/lib/entities/faction/CTownHandler.cpp index b5f84b2de..badfa92e9 100644 --- a/lib/entities/faction/CTownHandler.cpp +++ b/lib/entities/faction/CTownHandler.cpp @@ -373,6 +373,9 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons else ret->upgrade = BuildingID::NONE; + if (ret->town->buildings[ret->bid] != nullptr) + logMod->error("Mod %s, faction %s: detected multiple town buildings with ID %d", source.getModScope(), stringID, ret->bid.getNum()); + ret->town->buildings[ret->bid].reset(ret); for(const auto & element : source["marketModes"].Vector()) { From bc96515cec3befea82b97771ca73780e3d00894e Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Fri, 1 Aug 2025 18:21:02 +0300 Subject: [PATCH 10/11] Simplify blukMoveArmy logic, fix army transfer bugs --- AI/Nullkiller/Helpers/ArmyFormation.cpp | 6 +- lib/mapObjects/army/CCreatureSet.cpp | 14 --- lib/mapObjects/army/CCreatureSet.h | 1 - server/CGameHandler.cpp | 115 +++++++++++------------- 4 files changed, 53 insertions(+), 83 deletions(-) diff --git a/AI/Nullkiller/Helpers/ArmyFormation.cpp b/AI/Nullkiller/Helpers/ArmyFormation.cpp index 1e79e73e1..1eb230774 100644 --- a/AI/Nullkiller/Helpers/ArmyFormation.cpp +++ b/AI/Nullkiller/Helpers/ArmyFormation.cpp @@ -21,7 +21,7 @@ void ArmyFormation::rearrangeArmyForWhirlpool(const CGHeroInstance * hero) void ArmyFormation::addSingleCreatureStacks(const CGHeroInstance * hero) { - auto freeSlots = hero->getFreeSlotsQueue(); + auto freeSlots = hero->getFreeSlots(); while(!freeSlots.empty()) { @@ -37,8 +37,8 @@ void ArmyFormation::addSingleCreatureStacks(const CGHeroInstance * hero) break; } - cb->splitStack(hero, hero, weakestCreature->first, freeSlots.front(), 1); - freeSlots.pop(); + cb->splitStack(hero, hero, weakestCreature->first, freeSlots.back(), 1); + freeSlots.pop_back(); } } diff --git a/lib/mapObjects/army/CCreatureSet.cpp b/lib/mapObjects/army/CCreatureSet.cpp index d032ea3e2..bf3c49f0a 100644 --- a/lib/mapObjects/army/CCreatureSet.cpp +++ b/lib/mapObjects/army/CCreatureSet.cpp @@ -173,20 +173,6 @@ std::vector CCreatureSet::getFreeSlots(ui32 slotsAmount) const return freeSlots; } -std::queue CCreatureSet::getFreeSlotsQueue(ui32 slotsAmount) const -{ - std::queue freeSlots; - - for(ui32 i = 0; i < slotsAmount; i++) - { - auto slot = SlotID(i); - - if(!vstd::contains(stacks, slot)) - freeSlots.push(slot); - } - return freeSlots; -} - TMapCreatureSlot CCreatureSet::getCreatureMap() const { TMapCreatureSlot creatureMap; diff --git a/lib/mapObjects/army/CCreatureSet.h b/lib/mapObjects/army/CCreatureSet.h index d208a52b1..7fa7f21bc 100644 --- a/lib/mapObjects/army/CCreatureSet.h +++ b/lib/mapObjects/army/CCreatureSet.h @@ -117,7 +117,6 @@ public: SlotID getFreeSlot(ui32 slotsAmount = GameConstants::ARMY_SIZE) const; //returns first free slot std::vector getFreeSlots(ui32 slotsAmount = GameConstants::ARMY_SIZE) const; - std::queue getFreeSlotsQueue(ui32 slotsAmount = GameConstants::ARMY_SIZE) const; TMapCreatureSlot getCreatureMap() const; TCreatureQueue getCreatureQueue(const SlotID & exclude) const; diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index f6ef0d499..8b44cd34d 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -1714,83 +1714,68 @@ bool CGameHandler::bulkMoveArmy(ObjectInstanceID srcArmy, ObjectInstanceID destA if(!srcSlot.validSlot() && complain(complainInvalidSlot)) return false; - const auto * armySrc = dynamic_cast(gameInfo().getObjInstance(srcArmy)); - const CCreatureSet & setSrc = *armySrc; + if(!isAllowedExchange(srcArmy, destArmy)) + COMPLAIN_RET("That heroes cannot make any exchange!"); - if(!vstd::contains(setSrc.stacks, srcSlot) && complain(complainNoCreatures)) + const auto * armySrc = dynamic_cast(gameInfo().getObjInstance(srcArmy)); + const auto * armyDest = dynamic_cast(gameInfo().getObjInstance(destArmy)); + + if(!vstd::contains(armySrc->stacks, srcSlot) && complain(complainNoCreatures)) return false; - const auto * armyDest = dynamic_cast(gameInfo().getObjInstance(destArmy)); - const CCreatureSet & setDest = *armyDest; - auto freeSlots = setDest.getFreeSlotsQueue(); + auto freeSlots = armyDest->getFreeSlots(); + bool allTroopsMoved = true; - std::map> moves; - - auto srcQueue = setSrc.getCreatureQueue(srcSlot); // Exclude srcSlot, it should be moved last - auto slotsLeft = setSrc.stacksCount(); - auto destMap = setDest.getCreatureMap(); - TMapCreatureSlot::key_compare keyComp = destMap.key_comp(); - - while(!srcQueue.empty()) - { - auto pair = srcQueue.top(); - srcQueue.pop(); - - const auto * currCreature = pair.first; - auto currSlot = pair.second; - const auto quantity = setSrc.getStackCount(currSlot); - - const auto lb = destMap.lower_bound(currCreature); - const bool alreadyExists = (lb != destMap.end() && !(keyComp(currCreature, lb->first))); - - if(!alreadyExists) - { - if(freeSlots.empty()) - continue; - - auto currFreeSlot = freeSlots.front(); - freeSlots.pop(); - destMap.insert(lb, TMapCreatureSlot::value_type(currCreature, currFreeSlot)); - } - moves.insert(std::make_pair(currSlot, std::make_pair(destMap[currCreature], quantity))); - slotsLeft--; - } - if(slotsLeft == 1) - { - const auto * lastCreature = setSrc.getCreature(srcSlot); - auto slotToMove = SlotID(); - // Try to find a slot for last creature - if(destMap.find(lastCreature) == destMap.end()) - { - if(!freeSlots.empty()) - slotToMove = freeSlots.front(); - } - else - { - slotToMove = destMap[lastCreature]; - } - - if(slotToMove != SlotID()) - { - const bool needsLastStack = armySrc->needsLastStack(); - const auto quantity = setSrc.getStackCount(srcSlot) - (needsLastStack ? 1 : 0); - - if(quantity > 0) //0 may happen when we need last creature and we have exactly 1 amount of that creature - amount of "rest we can transfer" becomes 0 - moves.insert(std::make_pair(srcSlot, std::make_pair(slotToMove, quantity))); - } - } BulkRebalanceStacks bulkRS; - for(const auto & move : moves) + for (const auto & slot : armySrc->Slots()) { + auto targetSlot = armyDest->getSlotFor(slot.second->getCreature()); + + if (armyDest->slotEmpty(targetSlot)) + { + if (freeSlots.empty()) + { + allTroopsMoved = false; + continue; // no more free slots, but we might still have units that are present in both armies + } + + targetSlot = freeSlots.front(); + freeSlots.erase(freeSlots.begin()); + } + RebalanceStacks rs; rs.srcArmy = armySrc->id; rs.dstArmy = armyDest->id; - rs.srcSlot = move.first; - rs.dstSlot = move.second.first; - rs.count = move.second.second; + rs.srcSlot = slot.first; + rs.dstSlot = targetSlot; + rs.count = slot.second->getCount(); + bulkRS.moves.push_back(rs); } + + // all troops were moved, but we can't leave source hero without troops - undo movement of 1 unit from srcSlot + if (allTroopsMoved) + { + if (armySrc->getStack(srcSlot).getCount() == 1) + { + // slot only had 1 unit - remove this move completely + vstd::erase_if(bulkRS.moves, [srcSlot](const RebalanceStacks & move) + { + return move.srcSlot == srcSlot; + }); + } + else + { + // slot has multiple units - move all but one + for (auto & move : bulkRS.moves) + { + if (move.srcSlot == srcSlot) + move.count -= 1; + } + } + } + sendAndApply(bulkRS); return true; } From 9b61c57eef177cc74c6a71d1829fedee0008b789 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Fri, 1 Aug 2025 18:45:34 +0300 Subject: [PATCH 11/11] Slightly better fix for some map loading crashes --- lib/mapObjects/CGTownInstance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index d700f402c..8407872d6 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -706,7 +706,7 @@ void CGTownInstance::updateAppearance() std::string CGTownInstance::nodeName() const { - return "Town (" + getTown()->faction->getNameTranslated() + ") of " + getNameTranslated(); + return "Town at " + pos.toString(); } void CGTownInstance::updateMoraleBonusFromArmy()