Merge pull request #5962 from IvanSavenko/bugfixing

Fixes for recently reported issues

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();
 	}
 }
 

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<Component> artComponents;
 		for(const auto & artPack : pack.movingArtifacts)

client/windows/QuickRecruitmentWindow.cpp

@@ -103,26 +103,35 @@ void QuickRecruitmentWindow::maxAllCards(std::vector<std::shared_ptr<CreaturePur
 
 void QuickRecruitmentWindow::purchaseUnits()
 {
+	int freeSlotsLeft = town->getUpperArmy()->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)
+		{
+			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))
 		{
-			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())
+			if(freeSlotsLeft == 0)
 				continue;
-			onRecruit(crid, selected->slider->getValue());
+			freeSlotsLeft -= 1;
 		}
+
+		if(dstslot.validSlot())
+			GAME->interface()->cb->recruitCreatures(town, town->getUpperArmy(), crid, selected->slider->getValue(), level);
 	}
 	close();
 }

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;
 }
 

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);
 }
 

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())
 	{

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)
 	{

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()

lib/mapObjects/army/CCreatureSet.cpp

@@ -173,20 +173,6 @@ std::vector<SlotID> CCreatureSet::getFreeSlots(ui32 slotsAmount) const
 	return freeSlots;
 }
 
-std::queue<SlotID> CCreatureSet::getFreeSlotsQueue(ui32 slotsAmount) const
-{
-	std::queue<SlotID> 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;

lib/mapObjects/army/CCreatureSet.h

@@ -117,7 +117,6 @@ public:
 
 	SlotID getFreeSlot(ui32 slotsAmount = GameConstants::ARMY_SIZE) const; //returns first free slot
 	std::vector<SlotID> getFreeSlots(ui32 slotsAmount = GameConstants::ARMY_SIZE) const;
-	std::queue<SlotID> getFreeSlotsQueue(ui32 slotsAmount = GameConstants::ARMY_SIZE) const;
 
 	TMapCreatureSlot getCreatureMap() const;
 	TCreatureQueue getCreatureQueue(const SlotID & exclude) const;

lib/mapping/MapFormatH3M.cpp

@@ -2474,7 +2474,10 @@ std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readTown(const int3 & position,
 
 	std::optional<FactionID> faction;
 	if (objectTemplate->id == Obj::TOWN)
+	{
 		faction = FactionID(objectTemplate->subid);
+		object->subID = objectTemplate->subid;
+	}
 
 	bool hasName = reader->readBool();
 	if(hasName)

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();
 
@@ -1714,83 +1714,68 @@ bool CGameHandler::bulkMoveArmy(ObjectInstanceID srcArmy, ObjectInstanceID destA
 	if(!srcSlot.validSlot() && complain(complainInvalidSlot))
 		return false;
 
+	if(!isAllowedExchange(srcArmy, destArmy))
+		COMPLAIN_RET("That heroes cannot make any exchange!");
+
 	const auto * armySrc = dynamic_cast<const CArmedInstance*>(gameInfo().getObjInstance(srcArmy));
-	const CCreatureSet & setSrc = *armySrc;
+	const auto * armyDest = dynamic_cast<const CArmedInstance*>(gameInfo().getObjInstance(destArmy));
 
-	if(!vstd::contains(setSrc.stacks, srcSlot) && complain(complainNoCreatures))
+	if(!vstd::contains(armySrc->stacks, srcSlot) && complain(complainNoCreatures))
 		return false;
 
-	const auto * armyDest = dynamic_cast<const CArmedInstance*>(gameInfo().getObjInstance(destArmy));
-	const CCreatureSet & setDest = *armyDest;
-	auto freeSlots = setDest.getFreeSlotsQueue();
-
-	std::map<SlotID, std::pair<SlotID, TQuantity>> moves;
+	auto freeSlots = armyDest->getFreeSlots();
+	bool allTroopsMoved = true;
 
-	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();
+	BulkRebalanceStacks bulkRS;
 
-	while(!srcQueue.empty())
+	for (const auto & slot : armySrc->Slots())
 	{
-		auto pair = srcQueue.top();
-		srcQueue.pop();
-
-		const auto * currCreature = pair.first;
-		auto currSlot = pair.second;
-		const auto quantity = setSrc.getStackCount(currSlot);
+		auto targetSlot = armyDest->getSlotFor(slot.second->getCreature());
 
-		const auto lb = destMap.lower_bound(currCreature);
-		const bool alreadyExists = (lb != destMap.end() && !(keyComp(currCreature, lb->first)));
-
-		if(!alreadyExists)
+		if (armyDest->slotEmpty(targetSlot))
 		{
-			if(freeSlots.empty())
-				continue;
+			if (freeSlots.empty())
+			{
+				allTroopsMoved = false;
+				continue; // no more free slots, but we might still have units that are present in both armies
+			}
 
-			auto currFreeSlot = freeSlots.front();
-			freeSlots.pop();
-			destMap.insert(lb, TMapCreatureSlot::value_type(currCreature, currFreeSlot));
+			targetSlot = freeSlots.front();
+			freeSlots.erase(freeSlots.begin());
 		}
-		moves.insert(std::make_pair(currSlot, std::make_pair(destMap[currCreature], quantity)));
-		slotsLeft--;
+
+		RebalanceStacks rs;
+		rs.srcArmy = armySrc->id;
+		rs.dstArmy = armyDest->id;
+		rs.srcSlot = slot.first;
+		rs.dstSlot = targetSlot;
+		rs.count = slot.second->getCount();
+
+		bulkRS.moves.push_back(rs);
 	}
-	if(slotsLeft == 1)
+
+	// all troops were moved, but we can't leave source hero without troops - undo movement of 1 unit from srcSlot
+	if (allTroopsMoved)
 	{
-		const auto * lastCreature = setSrc.getCreature(srcSlot);
-		auto slotToMove = SlotID();
-		// Try to find a slot for last creature
-		if(destMap.find(lastCreature) == destMap.end())
+		if (armySrc->getStack(srcSlot).getCount() == 1)
 		{
-			if(!freeSlots.empty())
-				slotToMove = freeSlots.front();
+			// slot only had 1 unit - remove this move completely
+			vstd::erase_if(bulkRS.moves, [srcSlot](const RebalanceStacks & move)
+			{
+				return move.srcSlot == srcSlot;
+			});
 		}
 		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)));
+			// slot has multiple units - move all but one
+			for (auto & move : bulkRS.moves)
+			{
+				if (move.srcSlot == srcSlot)
+					move.count -= 1;
+			}
 		}
 	}
-	BulkRebalanceStacks bulkRS;
 
-	for(const auto & move : moves)
-	{
-		RebalanceStacks rs;
-		rs.srcArmy = armySrc->id;
-		rs.dstArmy = armyDest->id;
-		rs.srcSlot = move.first;
-		rs.dstSlot = move.second.first;
-		rs.count = move.second.second;
-		bulkRS.moves.push_back(rs);
-	}
 	sendAndApply(bulkRS);
 	return true;
 }
@@ -2947,6 +2932,8 @@ bool CGameHandler::assembleArtifacts(ObjectInstanceID heroID, ArtifactPosition a
 		sendAndApply(da);
 	}
 
+	checkVictoryLossConditionsForPlayer(hero->getOwner());
+
 	return true;
 }