Removed buggy and poorly designed fromString method

Use VLC->modh directly with proper parameters instead

lib/GameConstants.cpp

@@ -305,44 +305,14 @@ bool operator<(const BattleField & l, const BattleField & r)
 	return l.num < r.num;
 }
 
-BattleField::operator std::string() const
-{
-	return getInfo()->identifier;
-}
-
 const BattleFieldInfo * BattleField::getInfo() const
 {
 	return VLC->battlefields()->getById(*this);
 }
 
-BattleField BattleField::fromString(const std::string & identifier)
-{
-	auto rawId = VLC->modh->identifiers.getIdentifier(CModHandler::scopeBuiltin(), "battlefield", identifier);
-
-	if(rawId)
-		return BattleField(rawId.value());
-	else
-		return BattleField::NONE;
-}
-		
 const ObstacleInfo * Obstacle::getInfo() const
 {
 	return VLC->obstacles()->getById(*this);
 }
 
-Obstacle::operator std::string() const
-{
-	return getInfo()->identifier;
-}
-
-Obstacle Obstacle::fromString(const std::string & identifier)
-{
-	auto rawId = VLC->modh->identifiers.getIdentifier(CModHandler::scopeBuiltin(), "obstacle", identifier);
-
-	if(rawId)
-		return Obstacle(rawId.value());
-	else
-		return Obstacle(-1);
-}
-
 VCMI_LIB_NAMESPACE_END

lib/GameConstants.h

@@ -1292,10 +1292,7 @@ class BattleField : public BaseForID<BattleField, si32>
 	DLL_LINKAGE friend bool operator!=(const BattleField & l, const BattleField & r);
 	DLL_LINKAGE friend bool operator<(const BattleField & l, const BattleField & r);
 
-	DLL_LINKAGE operator std::string() const;
 	DLL_LINKAGE const BattleFieldInfo * getInfo() const;
-
-	DLL_LINKAGE static BattleField fromString(const std::string & identifier);
 };
 
 enum class EBoatId : int32_t
@@ -1336,8 +1333,6 @@ class Obstacle : public BaseForID<Obstacle, si32>
 	INSTID_LIKE_CLASS_COMMON(Obstacle, si32)
 
 	DLL_LINKAGE const ObstacleInfo * getInfo() const;
-	DLL_LINKAGE operator std::string() const;
-	DLL_LINKAGE static Obstacle fromString(const std::string & identifier);
 };
 
 enum class ESpellSchool: int8_t

lib/gameState/CGameState.cpp

@@ -1244,12 +1244,11 @@ BattleField CGameState::battleGetBattlefieldType(int3 tile, CRandomGenerator & r
 	}
 
 	if(map->isCoastalTile(tile)) //coastal tile is always ground
-		return BattleField::fromString("sand_shore");
+		return BattleField(*VLC->modh->identifiers.getIdentifier("core", "battlefield", "sand_shore"));
 	
 	return BattleField(*RandomGeneratorUtil::nextItem(t.terType->battleFields, rand));
 }
 
-
 void CGameState::fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out) const
 {
 	assert(obj);

lib/mapObjectConstructors/AObjectTypeHandler.cpp

@@ -13,6 +13,7 @@
 
 #include "IObjectInfo.h"
 #include "../CGeneralTextHandler.h"
+#include "../CModHandler.h"
 #include "../VCMI_Lib.h"
 #include "../mapObjects/CGObjectInstance.h"
 #include "../mapObjects/ObjectTemplate.h"
@@ -95,10 +96,15 @@ void AObjectTypeHandler::init(const JsonNode & input)
 	else
 		aiValue = static_cast<std::optional<si32>>(input["aiValue"].Integer());
 
-	if(input["battleground"].getType() == JsonNode::JsonType::DATA_STRING)
+	battlefield = BattleField::NONE;
-		battlefield = input["battleground"].String();
+
-	else
+	if(!input["battleground"].isNull())
-		battlefield = std::nullopt;
+	{
+		VLC->modh->identifiers.requestIdentifier("battlefield", input["battleground"], [this](int32_t identifier)
+		{
+			battlefield = BattleField(identifier);
+		});
+	}
 
 	initTypeData(input);
 }
@@ -175,7 +181,7 @@ std::vector<std::shared_ptr<const ObjectTemplate>> AObjectTypeHandler::getTempla
 
 BattleField AObjectTypeHandler::getBattlefield() const
 {
-	return battlefield ? BattleField::fromString(battlefield.value()) : BattleField::NONE;
+	return battlefield;
 }
 
 std::vector<std::shared_ptr<const ObjectTemplate>>AObjectTypeHandler::getTemplates(TerrainId terrainType) const

lib/mapObjectConstructors/AObjectTypeHandler.h

@@ -34,7 +34,7 @@ class DLL_LINKAGE AObjectTypeHandler : public boost::noncopyable
 	SObjectSounds sounds;
 
 	std::optional<si32> aiValue;
-	std::optional<std::string> battlefield;
+	BattleField battlefield;
 
 	std::string modScope;
 	std::string typeName;

server/CGameHandler.cpp

@@ -2113,7 +2113,7 @@ void CGameHandler::setupBattle(int3 tile, const CArmedInstance *armies[2], const
 
 	BattleField terType = gs->battleGetBattlefieldType(tile, getRandomGenerator());
 	if (heroes[0] && heroes[0]->boat && heroes[1] && heroes[1]->boat)
-		terType = BattleField::fromString("ship_to_ship");
+		terType = BattleField(*VLC->modh->identifiers.getIdentifier("core", "battlefield", "ship_to_ship"));
 
 	//send info about battles
 	BattleStart bs;

test/game/CGameStateTest.cpp

@@ -195,7 +195,7 @@ public:
 		const auto & t = *gameCallback->getTile(tile);
 
 		auto terrain = t.terType->getId();
-		BattleField terType = BattleField::fromString("grass_hills");
+		BattleField terType(0);
 
 		//send info about battles
 

test/mock/BattleFake.cpp

@@ -94,7 +94,7 @@ void BattleFake::setupEmptyBattlefield()
 {
 	EXPECT_CALL(*this, getDefendedTown()).WillRepeatedly(Return(nullptr));
 	EXPECT_CALL(*this, getAllObstacles()).WillRepeatedly(Return(IBattleInfo::ObstacleCList()));
-	EXPECT_CALL(*this, getBattlefieldType()).WillRepeatedly(Return(BattleField::fromString("grass_hills")));
+	EXPECT_CALL(*this, getBattlefieldType()).WillRepeatedly(Return(BattleField(0)));
 }
 
 #if SCRIPTING_ENABLED