Merge pull request #4157 from IvanSavenko/bugfixing

[1.5.3] Bugfixing

client/CPlayerInterface.cpp

@@ -1093,18 +1093,20 @@ void CPlayerInterface::showMapObjectSelectDialog(QueryID askID, const Component
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 
-	std::vector<ObjectInstanceID> tmpObjects;
-	if(objects.size() && dynamic_cast<const CGTownInstance *>(cb->getObj(objects[0])))
-	{
-		// sorting towns (like in client)
-		std::vector <const CGTownInstance*> Towns = LOCPLINT->localState->getOwnedTowns();
-		for(auto town : Towns)
-			for(auto item : objects)
-				if(town == cb->getObj(item))
-					tmpObjects.push_back(item);
-	}
-	else // other object list than town
-		tmpObjects = objects;
+	std::vector<ObjectInstanceID> objectGuiOrdered = objects;
+
+	std::map<ObjectInstanceID, int> townOrder;
+	auto ownedTowns = localState->getOwnedTowns();
+
+	for (int i = 0; i < ownedTowns.size(); ++i)
+		townOrder[ownedTowns[i]->id] = i;
+
+	auto townComparator = [&townOrder](const ObjectInstanceID & left, const ObjectInstanceID & right){
+		uint32_t leftIndex= townOrder.count(left) ? townOrder.at(left) : std::numeric_limits<uint32_t>::max();
+		uint32_t rightIndex = townOrder.count(right) ? townOrder.at(right) : std::numeric_limits<uint32_t>::max();
+		return leftIndex < rightIndex;
+	};
+	std::stable_sort(objectGuiOrdered.begin(), objectGuiOrdered.end(), townComparator);
 
 	auto selectCallback = [=](int selection)
 	{
@@ -1120,9 +1122,9 @@ void CPlayerInterface::showMapObjectSelectDialog(QueryID askID, const Component
 	const std::string localDescription = description.toString();
 
 	std::vector<int> tempList;
-	tempList.reserve(tmpObjects.size());
+	tempList.reserve(objectGuiOrdered.size());
 
-	for(auto item : tmpObjects)
+	for(auto item : objectGuiOrdered)
 		tempList.push_back(item.getNum());
 
 	CComponent localIconC(icon);
@@ -1131,7 +1133,7 @@ void CPlayerInterface::showMapObjectSelectDialog(QueryID askID, const Component
 	localIconC.removeChild(localIcon.get(), false);
 
 	std::vector<std::shared_ptr<IImage>> images;
-	for(auto & obj : tmpObjects)
+	for(auto & obj : objectGuiOrdered)
 	{
 		if(!settings["general"]["enableUiEnhancements"].Bool())
 			break;
@@ -1146,8 +1148,8 @@ void CPlayerInterface::showMapObjectSelectDialog(QueryID askID, const Component
 
 	auto wnd = std::make_shared<CObjectListWindow>(tempList, localIcon, localTitle, localDescription, selectCallback, 0, images);
 	wnd->onExit = cancelCallback;
-	wnd->onPopup = [this, tmpObjects](int index) { CRClickPopup::createAndPush(cb->getObj(tmpObjects[index]), GH.getCursorPosition()); };
-	wnd->onClicked = [this, tmpObjects](int index) { adventureInt->centerOnObject(cb->getObj(tmpObjects[index])); GH.windows().totalRedraw(); };
+	wnd->onPopup = [this, objectGuiOrdered](int index) { CRClickPopup::createAndPush(cb->getObj(objectGuiOrdered[index]), GH.getCursorPosition()); };
+	wnd->onClicked = [this, objectGuiOrdered](int index) { adventureInt->centerOnObject(cb->getObj(objectGuiOrdered[index])); GH.windows().totalRedraw(); };
 	GH.windows().pushWindow(wnd);
 }
 

lib/IGameCallback.cpp

@@ -158,17 +158,17 @@ void CPrivilegedInfoCallback::pickAllowedArtsSet(std::vector<const CArtifact *>
 
 void CPrivilegedInfoCallback::getAllowedSpells(std::vector<SpellID> & out, std::optional<ui16> level)
 {
-	for (ui32 i = 0; i < gs->map->allowedSpells.size(); i++) //spellh size appears to be greater (?)
+	for (auto const & spellID : gs->map->allowedSpells)
 	{
-		const spells::Spell * spell = SpellID(i).toSpell();
+		const auto * spell = spellID.toEntity(VLC);
 
-		if (!isAllowed(spell->getId()))
+		if (!isAllowed(spellID))
 			continue;
 
 		if (level.has_value() && spell->getLevel() != level)
 			continue;
 
-		out.push_back(spell->getId());
+		out.push_back(spellID);
 	}
 }
 

lib/mapObjects/CGTownBuilding.cpp

@@ -181,13 +181,14 @@ void COPWBonus::onHeroVisit (const CGHeroInstance * h) const
 			if(visitors.empty())
 			{
 				if(h->mana < h->manaLimit() * 2)
+				{
 					cb->setManaPoints (heroID, 2 * h->manaLimit());
-				//TODO: investigate line below
-				//cb->setObjProperty (town->id, ObjProperty::VISITED, true);
-				iw.text.appendRawString(getVisitingBonusGreeting());
-				cb->showInfoDialog(&iw);
-				//extra visit penalty if hero alredy had double mana points (or even more?!)
-				town->addHeroToStructureVisitors(h, indexOnTV);
+					//TODO: investigate line below
+					//cb->setObjProperty (town->id, ObjProperty::VISITED, true);
+					iw.text.appendRawString(getVisitingBonusGreeting());
+					cb->showInfoDialog(&iw);
+					town->addHeroToStructureVisitors(h, indexOnTV);
+				}
 			}
 			break;
 		}

lib/mapObjects/CQuest.cpp

@@ -141,6 +141,8 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 
 void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 {
+	// FIXME: this should be part of 'reward', and not hacking into limiter state that should only limit access to such reward
+
 	for(auto & elem : mission.artifacts)
 	{
 		if(h->hasArt(elem))
@@ -164,9 +166,9 @@ void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 			}
 		}
 	}
-			
+
 	cb->takeCreatures(h->id, mission.creatures);
-	cb->giveResources(h->getOwner(), mission.resources);
+	cb->giveResources(h->getOwner(), -mission.resources);
 }
 
 void CQuest::addTextReplacements(IGameCallback * cb, MetaString & text, std::vector<Component> & components) const

lib/mapObjects/MiscObjects.cpp

@@ -263,6 +263,9 @@ void CGResource::pickRandomObject(CRandomGenerator & rand)
 		ID = Obj::RESOURCE;
 		subID = rand.nextInt(EGameResID::WOOD, EGameResID::GOLD);
 		setType(ID, subID);
+
+		if (subID == EGameResID::GOLD && amount != CGResource::RANDOM_AMOUNT)
+			amount *= CGResource::GOLD_AMOUNT_MULTIPLIER;
 	}
 }
 
@@ -275,7 +278,7 @@ void CGResource::initObj(CRandomGenerator & rand)
 		switch(resourceID().toEnum())
 		{
 		case EGameResID::GOLD:
-			amount = rand.nextInt(5, 10) * 100;
+			amount = rand.nextInt(5, 10) * CGResource::GOLD_AMOUNT_MULTIPLIER;
 			break;
 		case EGameResID::WOOD: case EGameResID::ORE:
 			amount = rand.nextInt(6, 10);

lib/mapObjects/MiscObjects.h

@@ -122,8 +122,9 @@ class DLL_LINKAGE CGResource : public CArmedInstance
 public:
 	using CArmedInstance::CArmedInstance;
 
-	static constexpr ui32 RANDOM_AMOUNT = 0;
-	ui32 amount = RANDOM_AMOUNT; //0 if random
+	static constexpr uint32_t RANDOM_AMOUNT = 0;
+	static constexpr uint32_t GOLD_AMOUNT_MULTIPLIER = 100;
+	uint32_t amount = RANDOM_AMOUNT; //0 if random
 	
 	MetaString message;
 

lib/mapping/MapFormatH3M.cpp

@@ -1315,7 +1315,7 @@ CGObjectInstance * CMapLoaderH3M::readResource(const int3 & mapPosition, std::sh
 	if(GameResID(objectTemplate->subid) == GameResID(EGameResID::GOLD))
 	{
 		// Gold is multiplied by 100.
-		object->amount *= 100;
+		object->amount *= CGResource::GOLD_AMOUNT_MULTIPLIER;
 	}
 	reader->skipZero(4);
 	return object;

lib/rmg/RmgMap.cpp

@@ -368,12 +368,7 @@ ui32 RmgMap::getTotalZoneCount() const
 bool RmgMap::isAllowedSpell(const SpellID & sid) const
 {
 	assert(sid.getNum() >= 0);
-	if (sid.getNum() < mapInstance->allowedSpells.size())
-	{
-		return mapInstance->allowedSpells.count(sid);
-	}
-	else
-		return false;
+	return mapInstance->allowedSpells.count(sid);
 }
 
 void RmgMap::dump(bool zoneId) const