From e733b55c90ec7c714ecc2dc7c1cb6fefb367063b Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 25 Jul 2023 22:36:45 +0300 Subject: [PATCH] Removed buggy and poorly designed fromString method Use VLC->modh directly with proper parameters instead --- lib/GameConstants.cpp | 30 ------------------- lib/GameConstants.h | 5 ---- lib/gameState/CGameState.cpp | 3 +- .../AObjectTypeHandler.cpp | 16 ++++++---- .../AObjectTypeHandler.h | 2 +- server/CGameHandler.cpp | 2 +- test/game/CGameStateTest.cpp | 2 +- test/mock/BattleFake.cpp | 2 +- 8 files changed, 16 insertions(+), 46 deletions(-) diff --git a/lib/GameConstants.cpp b/lib/GameConstants.cpp index c440277de..5a1ffe19a 100644 --- a/lib/GameConstants.cpp +++ b/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 diff --git a/lib/GameConstants.h b/lib/GameConstants.h index 93a3085a3..ab2aaed89 100644 --- a/lib/GameConstants.h +++ b/lib/GameConstants.h @@ -1292,10 +1292,7 @@ class BattleField : public BaseForID 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 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 diff --git a/lib/gameState/CGameState.cpp b/lib/gameState/CGameState.cpp index 911584a30..4d6e572d4 100644 --- a/lib/gameState/CGameState.cpp +++ b/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); diff --git a/lib/mapObjectConstructors/AObjectTypeHandler.cpp b/lib/mapObjectConstructors/AObjectTypeHandler.cpp index a7c672533..53a19c83b 100644 --- a/lib/mapObjectConstructors/AObjectTypeHandler.cpp +++ b/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>(input["aiValue"].Integer()); - if(input["battleground"].getType() == JsonNode::JsonType::DATA_STRING) - battlefield = input["battleground"].String(); - else - battlefield = std::nullopt; + battlefield = BattleField::NONE; + + if(!input["battleground"].isNull()) + { + VLC->modh->identifiers.requestIdentifier("battlefield", input["battleground"], [this](int32_t identifier) + { + battlefield = BattleField(identifier); + }); + } initTypeData(input); } @@ -175,7 +181,7 @@ std::vector> AObjectTypeHandler::getTempla BattleField AObjectTypeHandler::getBattlefield() const { - return battlefield ? BattleField::fromString(battlefield.value()) : BattleField::NONE; + return battlefield; } std::vector>AObjectTypeHandler::getTemplates(TerrainId terrainType) const diff --git a/lib/mapObjectConstructors/AObjectTypeHandler.h b/lib/mapObjectConstructors/AObjectTypeHandler.h index f6776b0ac..ce35a9f82 100644 --- a/lib/mapObjectConstructors/AObjectTypeHandler.h +++ b/lib/mapObjectConstructors/AObjectTypeHandler.h @@ -34,7 +34,7 @@ class DLL_LINKAGE AObjectTypeHandler : public boost::noncopyable SObjectSounds sounds; std::optional aiValue; - std::optional battlefield; + BattleField battlefield; std::string modScope; std::string typeName; diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index a847ad764..9df39108a 100644 --- a/server/CGameHandler.cpp +++ b/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; diff --git a/test/game/CGameStateTest.cpp b/test/game/CGameStateTest.cpp index ded6e8baa..ae9bd2af0 100644 --- a/test/game/CGameStateTest.cpp +++ b/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 diff --git a/test/mock/BattleFake.cpp b/test/mock/BattleFake.cpp index b1e2af5c4..a2f5a9337 100644 --- a/test/mock/BattleFake.cpp +++ b/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