From 586620a290f6c681448b1530c0531ffccca42960 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 1 Apr 2025 17:21:17 +0300 Subject: [PATCH] Basic code review, remove unused code from serializers --- client/Client.cpp | 2 +- lib/CGameInfoCallback.h | 2 +- lib/CMakeLists.txt | 2 -- lib/CPlayerState.cpp | 4 ---- lib/CPlayerState.h | 1 - lib/gameState/CGameState.cpp | 5 ----- lib/networkPacks/NetPacksLib.cpp | 4 ++-- lib/serializer/BinaryDeserializer.cpp | 21 --------------------- lib/serializer/BinaryDeserializer.h | 8 +++++--- lib/serializer/BinarySerializer.cpp | 19 ------------------- lib/serializer/BinarySerializer.h | 5 ++++- lib/serializer/CSaveFile.cpp | 9 --------- lib/serializer/CSaveFile.h | 1 - lib/serializer/CSerializer.h | 25 +++++-------------------- server/CGameHandler.cpp | 2 +- 15 files changed, 19 insertions(+), 91 deletions(-) delete mode 100644 lib/serializer/BinaryDeserializer.cpp delete mode 100644 lib/serializer/BinarySerializer.cpp diff --git a/client/Client.cpp b/client/Client.cpp index 284af6991..5bb01593d 100644 --- a/client/Client.cpp +++ b/client/Client.cpp @@ -227,7 +227,7 @@ void CClient::initPlayerEnvironments() logNetwork->info("Preparing environment for player %s", color.toString()); playerEnvironments[color] = std::make_shared(color, this, std::make_shared(gamestate, color, this)); - if(!hasHumanPlayer && gameState()->players[color].isHuman()) + if(!hasHumanPlayer && gameState()->players.at(color).isHuman()) hasHumanPlayer = true; } diff --git a/lib/CGameInfoCallback.h b/lib/CGameInfoCallback.h index 0ef9083e4..d54f5e9b6 100644 --- a/lib/CGameInfoCallback.h +++ b/lib/CGameInfoCallback.h @@ -245,7 +245,7 @@ public: virtual int getHeroSerial(const CGHeroInstance * hero, bool includeGarrisoned=true) const; virtual const CGTownInstance* getTownBySerial(int serialId) const; // serial id is [0, number of towns) virtual const CGHeroInstance* getHeroBySerial(int serialId, bool includeGarrisoned=true) const; // serial id is [0, number of heroes) - virtual std::vector getHeroesInfo() const; //true -> only owned; false -> all visible + virtual std::vector getHeroesInfo() const; virtual std::vector getMyObjects() const; //returns all objects flagged by belonging player virtual std::vector getMyQuests() const; diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index bb0ae5402..125bb9d2d 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -218,8 +218,6 @@ set(lib_MAIN_SRCS rmg/modificators/TerrainPainter.cpp rmg/MapProxy.cpp - serializer/BinaryDeserializer.cpp - serializer/BinarySerializer.cpp serializer/CLoadFile.cpp serializer/CMemorySerializer.cpp serializer/Connection.cpp diff --git a/lib/CPlayerState.cpp b/lib/CPlayerState.cpp index 751250a7c..fd7390707 100644 --- a/lib/CPlayerState.cpp +++ b/lib/CPlayerState.cpp @@ -21,10 +21,6 @@ VCMI_LIB_NAMESPACE_BEGIN -PlayerState::PlayerState() - :PlayerState(nullptr) -{} - PlayerState::PlayerState(IGameCallback *cb) : CBonusSystemNode(PLAYER) , GameCallbackHolder(cb) diff --git a/lib/CPlayerState.h b/lib/CPlayerState.h index 499c098a6..45724cc84 100644 --- a/lib/CPlayerState.h +++ b/lib/CPlayerState.h @@ -75,7 +75,6 @@ public: TurnTimerInfo turnTimer; PlayerState(IGameCallback *cb); - PlayerState(); ~PlayerState(); std::string nodeName() const override; diff --git a/lib/gameState/CGameState.cpp b/lib/gameState/CGameState.cpp index dd5b46129..733cf3c2f 100644 --- a/lib/gameState/CGameState.cpp +++ b/lib/gameState/CGameState.cpp @@ -1573,11 +1573,6 @@ void CGameState::buildBonusSystemTree() void CGameState::deserializationFix() { - assert(cb != nullptr); - - for(auto & player : players) - player.second.cb = cb; - buildGlobalTeamPlayerTree(); attachArmedObjects(); diff --git a/lib/networkPacks/NetPacksLib.cpp b/lib/networkPacks/NetPacksLib.cpp index 0305f6be1..06d141d2b 100644 --- a/lib/networkPacks/NetPacksLib.cpp +++ b/lib/networkPacks/NetPacksLib.cpp @@ -2418,13 +2418,13 @@ void PlayerEndsTurn::applyGs(CGameState *gs) void DaysWithoutTown::applyGs(CGameState *gs) { - auto & playerState = gs->players[player]; + auto & playerState = gs->players.at(player); playerState.daysWithoutCastle = daysWithoutCastle; } void TurnTimeUpdate::applyGs(CGameState *gs) { - auto & playerState = gs->players[player]; + auto & playerState = gs->players.at(player); playerState.turnTimer = turnTimer; } diff --git a/lib/serializer/BinaryDeserializer.cpp b/lib/serializer/BinaryDeserializer.cpp deleted file mode 100644 index 94630bec8..000000000 --- a/lib/serializer/BinaryDeserializer.cpp +++ /dev/null @@ -1,21 +0,0 @@ -/* - * BinaryDeserializer.cpp, part of VCMI engine - * - * Authors: listed in file AUTHORS in main folder - * - * License: GNU General Public License v2.0 or later - * Full text of license available in license.txt file, in main folder - * - */ -#include "StdInc.h" -#include "BinaryDeserializer.h" - -VCMI_LIB_NAMESPACE_BEGIN - -BinaryDeserializer::BinaryDeserializer(IBinaryReader * r): CLoaderBase(r) -{ - version = Version::NONE; - reverseEndianness = false; -} - -VCMI_LIB_NAMESPACE_END diff --git a/lib/serializer/BinaryDeserializer.h b/lib/serializer/BinaryDeserializer.h index 9ce6aa879..cb9ea68d7 100644 --- a/lib/serializer/BinaryDeserializer.h +++ b/lib/serializer/BinaryDeserializer.h @@ -55,8 +55,8 @@ class BinaryDeserializer : public CLoaderBase public: using Version = ESerializationVersion; - bool reverseEndianness; //if source has different endianness than us, we reverse bytes - Version version; + bool reverseEndianness = false; //if source has different endianness than us, we reverse bytes + Version version = Version::NONE; std::vector loadedStrings; std::map loadedPointers; @@ -71,7 +71,9 @@ public: return version >= what; }; - DLL_LINKAGE BinaryDeserializer(IBinaryReader * r); + BinaryDeserializer(IBinaryReader * r) + : CLoaderBase(r) + {} template BinaryDeserializer & operator&(T & t) diff --git a/lib/serializer/BinarySerializer.cpp b/lib/serializer/BinarySerializer.cpp deleted file mode 100644 index 76d358100..000000000 --- a/lib/serializer/BinarySerializer.cpp +++ /dev/null @@ -1,19 +0,0 @@ -/* - * BinarySerializer.cpp, part of VCMI engine - * - * Authors: listed in file AUTHORS in main folder - * - * License: GNU General Public License v2.0 or later - * Full text of license available in license.txt file, in main folder - * - */ -#include "StdInc.h" -#include "BinarySerializer.h" - -VCMI_LIB_NAMESPACE_BEGIN - -BinarySerializer::BinarySerializer(IBinaryWriter * w): CSaverBase(w) -{ -} - -VCMI_LIB_NAMESPACE_END diff --git a/lib/serializer/BinarySerializer.h b/lib/serializer/BinarySerializer.h index 0a6c576a9..954e8ef6c 100644 --- a/lib/serializer/BinarySerializer.h +++ b/lib/serializer/BinarySerializer.h @@ -69,7 +69,10 @@ public: return version >= what; }; - DLL_LINKAGE BinarySerializer(IBinaryWriter * w); + BinarySerializer(IBinaryWriter * w) + : CSaverBase(w) + { + } template BinarySerializer & operator&(const T & t) diff --git a/lib/serializer/CSaveFile.cpp b/lib/serializer/CSaveFile.cpp index d406342b4..6c62d3248 100644 --- a/lib/serializer/CSaveFile.cpp +++ b/lib/serializer/CSaveFile.cpp @@ -49,15 +49,6 @@ void CSaveFile::openNextFile(const boost::filesystem::path &fname) } } -void CSaveFile::reportState(vstd::CLoggerBase * out) -{ - out->debug("CSaveFile"); - if(sfile.get() && *sfile) - { - out->debug("\tOpened %s \tPosition: %d", fName, sfile->tellp()); - } -} - void CSaveFile::clear() { fName.clear(); diff --git a/lib/serializer/CSaveFile.h b/lib/serializer/CSaveFile.h index 857b7f159..ebe3f563b 100644 --- a/lib/serializer/CSaveFile.h +++ b/lib/serializer/CSaveFile.h @@ -27,7 +27,6 @@ public: void openNextFile(const boost::filesystem::path &fname); //throws! void clear(); - void reportState(vstd::CLoggerBase * out) override; void putMagicBytes(const std::string &text); diff --git a/lib/serializer/CSerializer.h b/lib/serializer/CSerializer.h index f198e51b2..f9f9d3ccd 100644 --- a/lib/serializer/CSerializer.h +++ b/lib/serializer/CSerializer.h @@ -9,28 +9,10 @@ */ #pragma once -#include "../GameConstants.h" - VCMI_LIB_NAMESPACE_BEGIN const std::string SAVEGAME_MAGIC = "VCMISVG"; -class CHero; -class CGHeroInstance; -class CGObjectInstance; - -class CGameState; -class GameLibrary; -extern DLL_LINKAGE GameLibrary * LIBRARY; - -/// Base class for serializers capable of reading or writing data -class DLL_LINKAGE CSerializer : boost::noncopyable -{ -public: - virtual ~CSerializer() = default; - virtual void reportState(vstd::CLoggerBase * out){}; -}; - /// Helper to detect classes with user-provided serialize(S&, int version) method template struct is_serializeable @@ -45,16 +27,19 @@ struct is_serializeable }; /// Base class for deserializers -class DLL_LINKAGE IBinaryReader : public virtual CSerializer +class IBinaryReader { public: + virtual ~IBinaryReader() = default; virtual int read(std::byte * data, unsigned size) = 0; + virtual void reportState(vstd::CLoggerBase * out){}; }; /// Base class for serializers -class DLL_LINKAGE IBinaryWriter : public virtual CSerializer +class IBinaryWriter { public: + virtual ~IBinaryWriter() = default; virtual int write(const std::byte * data, unsigned size) = 0; }; diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index eadce1ec6..62e1cca9b 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -802,7 +802,7 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, EMovementMode moveme // not turn of that hero or player can't simply teleport hero (at least not with this function) if(!h || (asker != PlayerColor::NEUTRAL && movementMode != EMovementMode::STANDARD)) { - if(h && getStartInfo()->turnTimerInfo.isEnabled() && gameState()->players[h->getOwner()].turnTimer.turnTimer == 0) + if(h && getStartInfo()->turnTimerInfo.isEnabled() && gameState()->players.at(h->getOwner()).turnTimer.turnTimer == 0) return true; //timer expired, no error logGlobal->error("Illegal call to move hero!");