Issues fixed

lib/CCreatureSet.cpp

@@ -624,12 +624,13 @@ void CCreatureSet::armyChanged()
 
 }
 
-void CCreatureSet::serializeJson(JsonSerializeFormat & handler, const std::string & fieldName, const std::optional<int> fixedSize)
+void CCreatureSet::serializeJson(JsonSerializeFormat & handler, const std::string & armyFieldName, const std::optional<int> fixedSize)
 {
 	if(handler.saving && stacks.empty())
 		return;
 
-	auto a = handler.enterArray(fieldName);
+	handler.serializeEnum("formation", formation, NArmyFormation::names);
+	auto a = handler.enterArray(armyFieldName);
 
 
 	if(handler.saving)

lib/CCreatureSet.h

@@ -206,6 +206,11 @@ enum class EArmyFormation : uint8_t
 	TIGHT
 };
 
+namespace NArmyFormation
+{
+	static const std::vector<std::string> names{ "wide", "tight" };
+}
+
 class DLL_LINKAGE CCreatureSet : public IArmyDescriptor //seven combined creatures
 {
 	CCreatureSet(const CCreatureSet &) = delete;
@@ -284,7 +289,7 @@ public:
 		h & formation;
 	}
 
-	void serializeJson(JsonSerializeFormat & handler, const std::string & fieldName, const std::optional<int> fixedSize = std::nullopt);
+	void serializeJson(JsonSerializeFormat & handler, const std::string & armyFieldName, const std::optional<int> fixedSize = std::nullopt);
 
 	operator bool() const
 	{

lib/mapObjects/CArmedInstance.cpp

@@ -160,4 +160,10 @@ const IBonusBearer* CArmedInstance::getBonusBearer() const
 	return this;
 }
 
+void CArmedInstance::serializeJsonOptions(JsonSerializeFormat & handler)
+{
+	CGObjectInstance::serializeJsonOptions(handler);
+	CCreatureSet::serializeJson(handler, "army", 7);
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CArmedInstance.h

@@ -18,6 +18,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class BattleInfo;
 class CGameState;
+class JsonSerializeFormat;
 
 class DLL_LINKAGE CArmedInstance: public CGObjectInstance, public CBonusSystemNode, public CCreatureSet, public IConstBonusProvider
 {
@@ -49,6 +50,8 @@ public:
 		return this->tempOwner;
 	}
 	
+	void serializeJsonOptions(JsonSerializeFormat & handler) override;
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CGObjectInstance&>(*this);

lib/mapObjects/CGHeroInstance.cpp

@@ -1709,10 +1709,7 @@ void CGHeroInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 			setHeroTypeName(typeName);
 	}
 
-	static const std::vector<std::string> FORMATIONS  =	{ "wide", "tight" };
+	CArmedInstance::serializeJsonOptions(handler);
-
-	CCreatureSet::serializeJson(handler, "army", 7);
-	handler.serializeEnum("formation", formation, FORMATIONS);
 
 	{
 		static constexpr int NO_PATROLING = -1;

lib/mapObjects/CGPandoraBox.cpp

@@ -28,9 +28,15 @@ VCMI_LIB_NAMESPACE_BEGIN
 void CGPandoraBox::init()
 {
 	blockVisit = true;
+	configuration.info.emplace_back();
+	configuration.info.back().visitType = Rewardable::EEventType::EVENT_FIRST_VISIT;
 	
 	for(auto & i : configuration.info)
+	{
 		i.reward.removeObject = true;
+		if(!message.empty() && i.message.empty())
+			i.message = MetaString::createFromRawString(message);
+	}
 }
 
 void CGPandoraBox::initObj(CRandomGenerator & rand)
@@ -202,14 +208,17 @@ void CGPandoraBox::blockingDialogAnswered(const CGHeroInstance *hero, ui32 answe
 void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 {
 	CRewardableObject::serializeJsonOptions(handler);
+	
 	handler.serializeString("guardMessage", message);
 	
 	if(!handler.saving)
 	{
-		//backward compatibility
+		//backward compatibility for VCMI maps that use old Pandora Box format
-		CCreatureSet::serializeJson(handler, "guards", 7);
+		if(!handler.getCurrent()["guards"].Vector().empty())
-		configuration.info.emplace_back();
+			CCreatureSet::serializeJson(handler, "guards", 7);
-		Rewardable::VisitInfo & vinfo = configuration.info.back();
+		
+		bool hasSomething = false;
+		Rewardable::VisitInfo vinfo;
 		vinfo.visitType = Rewardable::EEventType::EVENT_FIRST_VISIT;
 				
 		handler.serializeInt("experience", vinfo.reward.heroExperience, 0);
@@ -230,6 +239,8 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 			for(int idx = 0; idx < vinfo.reward.primary.size(); idx ++)
 			{
 				handler.serializeInt(NPrimarySkill::names[idx], vinfo.reward.primary[idx], 0);
+				if(vinfo.reward.primary[idx])
+					hasSomething = true;
 			}
 		}
 		
@@ -261,6 +272,19 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 				vinfo.reward.secondary[rawId] = level;
 			}
 		}
+		
+		hasSomething = hasSomething
+		|| vinfo.reward.heroExperience
+		|| vinfo.reward.manaDiff
+		|| vinfo.reward.resources.nonZero()
+		|| !vinfo.reward.bonuses.empty()
+		|| !vinfo.reward.artifacts.empty()
+		|| !vinfo.reward.secondary.empty()
+		|| !vinfo.reward.artifacts.empty()
+		|| !vinfo.reward.creatures.empty();
+		
+		if(hasSomething)
+			configuration.info.push_back(vinfo);
 	}
 }
 

lib/mapObjects/CGTownInstance.cpp

@@ -1093,11 +1093,10 @@ void CGTownInstance::reset()
 
 void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 {
-	static const std::vector<std::string> FORMATIONS  =	{ "wide", "tight" };
-
 	CGObjectInstance::serializeJsonOwner(handler);
-	CCreatureSet::serializeJson(handler, "army", 7);
+	if(!handler.saving)
-	handler.serializeEnum("tightFormation", formation, FORMATIONS);
+		handler.serializeEnum("tightFormation", formation, NArmyFormation::names); //for old format
+	CArmedInstance::serializeJsonOptions(handler);
 	handler.serializeString("name", name);
 
 	{

lib/mapObjects/CQuest.cpp

@@ -799,7 +799,7 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 	
 	if(!handler.saving)
 	{
-		//backward compatibility
+		//backward compatibility for VCMI maps that use old SeerHut format
 		auto s = handler.enterStruct("reward");
 		const JsonNode & rewardsJson = handler.getCurrent();
 		

lib/mapObjects/CQuest.h

@@ -138,7 +138,7 @@ protected:
 	void afterAddToMapCommon(CMap * map) const;
 };
 
-class DLL_LINKAGE CGSeerHut : public CRewardableObject, public IQuestObject //army is used when giving reward
+class DLL_LINKAGE CGSeerHut : public CRewardableObject, public IQuestObject
 {
 public:
 	std::string seerName;

lib/mapObjects/MiscObjects.cpp

@@ -196,7 +196,7 @@ void CGMine::blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer) con
 
 void CGMine::serializeJsonOptions(JsonSerializeFormat & handler)
 {
-	CCreatureSet::serializeJson(handler, "army", 7);
+	CArmedInstance::serializeJsonOptions(handler);
 
 	if(isAbandoned())
 	{
@@ -316,7 +316,9 @@ void CGResource::blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer)
 
 void CGResource::serializeJsonOptions(JsonSerializeFormat & handler)
 {
-	CCreatureSet::serializeJson(handler, "guards", 7);
+	CArmedInstance::serializeJsonOptions(handler);
+	if(!handler.saving && !handler.getCurrent()["guards"].Vector().empty())
+		CCreatureSet::serializeJson(handler, "guards", 7);
 	handler.serializeInt("amount", amount, 0);
 	handler.serializeString("guardMessage", message);
 }
@@ -827,7 +829,9 @@ void CGArtifact::afterAddToMap(CMap * map)
 void CGArtifact::serializeJsonOptions(JsonSerializeFormat& handler)
 {
 	handler.serializeString("guardMessage", message);
-	CCreatureSet::serializeJson(handler, "guards" ,7);
+	CArmedInstance::serializeJsonOptions(handler);
+	if(!handler.saving && !handler.getCurrent()["guards"].Vector().empty())
+		CCreatureSet::serializeJson(handler, "guards", 7);
 
 	if(handler.saving && ID == Obj::SPELL_SCROLL)
 	{
@@ -1233,7 +1237,7 @@ void CGGarrison::serializeJsonOptions(JsonSerializeFormat& handler)
 {
 	handler.serializeBool("removableUnits", removableUnits);
 	serializeJsonOwner(handler);
-	CCreatureSet::serializeJson(handler, "army", 7);
+	CArmedInstance::serializeJsonOptions(handler);
 }
 
 void CGMagi::reset()

lib/rewardable/Limiter.cpp

@@ -133,56 +133,18 @@ void Rewardable::Limiter::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeInt("manaPoints", manaPoints);
 	handler.serializeIdArray("artifacts", artifacts);
 	handler.enterArray("creatures").serializeStruct(creatures);
-	{
+	handler.enterArray("primary").serializeArray(primary);
-		auto a = handler.enterArray("primary");
-		a.syncSize(primary);
-		for(int i = 0; i < primary.size(); ++i)
-			a.serializeInt(i, primary[i]);
-	}
-	
 	{
 		auto a = handler.enterArray("secondary");
-		std::vector<std::pair<std::string, std::string>> fieldValue;
+		std::vector<std::pair<SecondarySkill, si32>> fieldValue(secondary.begin(), secondary.end());
-		if(handler.saving)
+		a.serializeStruct<std::pair<SecondarySkill, si32>>(fieldValue, [](JsonSerializeFormat & h, std::pair<SecondarySkill, si32> & e)
 		{
-			for(auto & i : secondary)
+			h.serializeId("skill", e.first, SecondarySkill{}, VLC->skillh->decodeSkill, VLC->skillh->encodeSkill);
-			{
+			h.serializeId("level", e.second, 0, [](const std::string & i){return vstd::find_pos(NSecondarySkill::levels, i);}, [](si32 i){return NSecondarySkill::levels.at(i);});
-				auto key = VLC->skillh->encodeSkill(i.first);
+		});
-				auto value = NSecondarySkill::levels.at(i.second);
-				fieldValue.emplace_back(key, value);
-			}
-		}
 		a.syncSize(fieldValue);
-		for(int i = 0; i < fieldValue.size(); ++i)
+		secondary = std::map<SecondarySkill, si32>(fieldValue.begin(), fieldValue.end());
-		{
-			auto e = a.enterStruct(i);
-			e->serializeString("skill", fieldValue[i].first);
-			e->serializeString("level", fieldValue[i].second);
-		}
-		if(!handler.saving)
-		{
-			for(auto & i : fieldValue)
-			{
-				const int skillId = VLC->skillh->decodeSkill(i.first);
-				if(skillId < 0)
-				{
-					logGlobal->error("Invalid secondary skill %s", i.first);
-					continue;
-				}
-				
-				const int level = vstd::find_pos(NSecondarySkill::levels, i.second);
-				if(level < 0)
-				{
-					logGlobal->error("Invalid secondary skill level%s", i.second);
-					continue;
-				}
-				
-				secondary[SecondarySkill(skillId)] = level;
-			}
-				
-		}
 	}
-	
 	//sublimiters
 	auto serializeSublimitersList = [&handler](const std::string & field, LimitersList & container)
 	{

lib/rewardable/Reward.cpp

@@ -119,76 +119,28 @@ void Rewardable::Reward::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeIdArray("artifacts", artifacts);
 	handler.serializeIdArray("spells", spells);
 	handler.enterArray("creatures").serializeStruct(creatures);
-	{
+	handler.enterArray("primary").serializeArray(primary);
-		auto a = handler.enterArray("primary");
-		a.syncSize(primary);
-		for(int i = 0; i < primary.size(); ++i)
-			a.serializeInt(i, primary[i]);
-	}
-	
 	{
 		auto a = handler.enterArray("secondary");
-		std::vector<std::pair<std::string, std::string>> fieldValue;
+		std::vector<std::pair<SecondarySkill, si32>> fieldValue(secondary.begin(), secondary.end());
-		if(handler.saving)
+		a.serializeStruct<std::pair<SecondarySkill, si32>>(fieldValue, [](JsonSerializeFormat & h, std::pair<SecondarySkill, si32> & e)
 		{
-			for(auto & i : secondary)
+			h.serializeId("skill", e.first, SecondarySkill{}, VLC->skillh->decodeSkill, VLC->skillh->encodeSkill);
-			{
+			h.serializeId("level", e.second, 0, [](const std::string & i){return vstd::find_pos(NSecondarySkill::levels, i);}, [](si32 i){return NSecondarySkill::levels.at(i);});
-				auto key = VLC->skillh->encodeSkill(i.first);
+		});
-				auto value = NSecondarySkill::levels.at(i.second);
-				fieldValue.emplace_back(key, value);
-			}
-		}
 		a.syncSize(fieldValue);
-		for(int i = 0; i < fieldValue.size(); ++i)
+		secondary = std::map<SecondarySkill, si32>(fieldValue.begin(), fieldValue.end());
-		{
-			auto e = a.enterStruct(i);
-			e->serializeString("skill", fieldValue[i].first);
-			e->serializeString("level", fieldValue[i].second);
-		}
-		if(!handler.saving)
-		{
-			for(auto & i : fieldValue)
-			{
-				const int skillId = VLC->skillh->decodeSkill(i.first);
-				if(skillId < 0)
-				{
-					logGlobal->error("Invalid secondary skill %s", i.first);
-					continue;
-				}
-				
-				const int level = vstd::find_pos(NSecondarySkill::levels, i.second);
-				if(level < 0)
-				{
-					logGlobal->error("Invalid secondary skill level%s", i.second);
-					continue;
-				}
-				
-				secondary[SecondarySkill(skillId)] = level;
-			}
-				
-		}
 	}
 	
 	{
 		auto a = handler.enterArray("creaturesChange");
-		std::vector<std::pair<CreatureID, CreatureID>> fieldValue;
+		std::vector<std::pair<CreatureID, CreatureID>> fieldValue(creaturesChange.begin(), creaturesChange.end());
-		if(handler.saving)
+		a.serializeStruct<std::pair<CreatureID, CreatureID>>(fieldValue, [](JsonSerializeFormat & h, std::pair<CreatureID, CreatureID> & e)
 		{
-			for(auto & i : creaturesChange)
+			h.serializeId("creature", e.first, CreatureID{});
-				fieldValue.push_back(i);
+			h.serializeId("amount", e.second, CreatureID{});
-		}
+		});
-		a.syncSize(fieldValue);
+		creaturesChange = std::map<CreatureID, CreatureID>(fieldValue.begin(), fieldValue.end());
-		for(int i = 0; i < fieldValue.size(); ++i)
-		{
-			auto e = a.enterStruct(i);
-			e->serializeId("creature", fieldValue[i].first, CreatureID{});
-			e->serializeId("amount", fieldValue[i].second, CreatureID{});
-		}
-		if(!handler.saving)
-		{
-			for(auto & i : fieldValue)
-				creaturesChange[i.first] = i.second;
-		}
 	}
 	
 	{

lib/serializer/JsonSerializeFormat.h

@@ -74,19 +74,45 @@ public:
 	///String <-> Json string
 	void serializeString(const size_t index, std::string & value);
 
-	///vector of serializable <-> Json vector of structs
+	///vector of anything int-convertible <-> Json vector of integers
+	template<typename T>
+	void serializeArray(std::vector<T> & value)
+	{
+		syncSize(value, JsonNode::JsonType::DATA_STRUCT);
+
+		for(size_t idx = 0; idx < size(); idx++)
+			serializeInt(idx, value[idx]);
+	}
+	
+	///vector of strings <-> Json vector of strings
+	void serializeArray(std::vector<std::string> & value)
+	{
+		syncSize(value, JsonNode::JsonType::DATA_STRUCT);
+
+		for(size_t idx = 0; idx < size(); idx++)
+			serializeString(idx, value[idx]);
+	}
+	
+	///vector of anything with custom serializing function <-> Json vector of structs
 	template <typename Element>
-	void serializeStruct(std::vector<Element> & value)
+	void serializeStruct(std::vector<Element> & value, std::function<void(JsonSerializeFormat&, Element&)> serializer)
 	{
 		syncSize(value, JsonNode::JsonType::DATA_STRUCT);
 
 		for(size_t idx = 0; idx < size(); idx++)
 		{
 			auto s = enterStruct(idx);
-			value[idx].serializeJson(*owner);
+			serializer(*owner, value[idx]);
 		}
 	}
 	
+	///vector of serializable <-> Json vector of structs
+	template <typename Element>
+	void serializeStruct(std::vector<Element> & value)
+	{
+		serializeStruct<Element>(value, [](JsonSerializeFormat & h, Element & e){e.serializeJson(h);});
+	}
+
 	void resize(const size_t newSize);
 	void resize(const size_t newSize, JsonNode::JsonType type);
 	size_t size() const;

mapeditor/inspector/rewardswidget.cpp

@@ -208,7 +208,10 @@ bool RewardsWidget::commitChanges()
 	object.configuration.visitMode = ui->visitMode->currentIndex();
 	object.configuration.selectMode = ui->selectMode->currentIndex();
 	object.configuration.infoWindowType = EInfoWindowMode(ui->windowMode->currentIndex());
-	object.configuration.onSelect = MetaString::createFromRawString(ui->onSelectText->text().toStdString());
+	if(ui->onSelectText->text().isEmpty())
+		object.configuration.onSelect.clear();
+	else
+		object.configuration.onSelect = MetaString::createFromRawString(ui->onSelectText->text().toStdString());
 	object.configuration.canRefuse = ui->canRefuse->isChecked();
 	
 	//reset parameters
@@ -226,7 +229,10 @@ void RewardsWidget::saveCurrentVisitInfo(int index)
 {
 	auto & vinfo = object.configuration.info.at(index);
 	vinfo.visitType = Rewardable::EEventType::EVENT_FIRST_VISIT;
-	vinfo.message = MetaString::createFromRawString(ui->rewardMessage->text().toStdString());
+	if(ui->rewardMessage->text().isEmpty())
+		vinfo.message.clear();
+	else
+		vinfo.message = MetaString::createFromRawString(ui->rewardMessage->text().toStdString());
 	
 	vinfo.reward.heroLevel = ui->rHeroLevel->value();
 	vinfo.reward.heroExperience = ui->rHeroExperience->value();