Apply review remarks:

- avoid early returns
- rename cheatStr to enableCheatsStr
- rename canCheatModeBeOn() to areCheatsAllowed()
- trackCharKeyEvent return void
- hold cheatCmdTracker_ by value instead of unique_ptr

Author: kubaau

libs/s25main/CheatCommandTracker.cpp

@@ -11,20 +11,17 @@ auto makeCircularBuffer(const std::string& str)
 {
     return boost::circular_buffer<char>{cbegin(str), cend(str)};
 }
-const auto cheatStr = makeCircularBuffer("winter");
+const auto enableCheatsStr = makeCircularBuffer("winter");
 } // namespace
 
-CheatCommandTracker::CheatCommandTracker(Cheats& cheats) : cheats_(cheats), lastChars_(cheatStr.size()) {}
+CheatCommandTracker::CheatCommandTracker(Cheats& cheats) : cheats_(cheats), lastChars_(enableCheatsStr.size()) {}
 
 void CheatCommandTracker::trackKeyEvent(const KeyEvent& ke)
 {
     if(trackSpecialKeyEvent(ke))
-    {
         lastChars_.clear();
-        return;
-    }
-
-    trackCharKeyEvent(ke);
+    else
+        trackCharKeyEvent(ke);
 }
 
 void CheatCommandTracker::trackChatCommand(const std::string& cmd)
@@ -47,12 +44,10 @@ bool CheatCommandTracker::trackSpecialKeyEvent(const KeyEvent& ke)
     return true;
 }
 
-bool CheatCommandTracker::trackCharKeyEvent(const KeyEvent& ke)
+void CheatCommandTracker::trackCharKeyEvent(const KeyEvent& ke)
 {
     lastChars_.push_back(ke.c);
 
-    if(lastChars_ == cheatStr)
+    if(lastChars_ == enableCheatsStr)
         cheats_.toggleCheatMode();
-
-    return true;
 }

libs/s25main/CheatCommandTracker.h

@@ -20,7 +20,7 @@ class CheatCommandTracker
 
 private:
     bool trackSpecialKeyEvent(const KeyEvent& ke);
-    bool trackCharKeyEvent(const KeyEvent& ke);
+    void trackCharKeyEvent(const KeyEvent& ke);
 
     Cheats& cheats_;
     boost::circular_buffer<char> lastChars_;

libs/s25main/Cheats.cpp

@@ -3,58 +3,44 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "Cheats.h"
-#include "CheatCommandTracker.h"
 #include "network/GameClient.h"
 #include "world/GameWorldBase.h"
 
-Cheats::Cheats(GameWorldBase& world) : cheatCmdTracker_(std::make_unique<CheatCommandTracker>(*this)), world_(world) {}
+Cheats::Cheats(GameWorldBase& world) : cheatCmdTracker_(*this), world_(world) {}
 
 Cheats::~Cheats() = default;
 
 void Cheats::trackKeyEvent(const KeyEvent& ke)
 {
-    if(!canCheatModeBeOn())
-        return;
-
-    cheatCmdTracker_->trackKeyEvent(ke);
+    if(areCheatsAllowed())
+        cheatCmdTracker_.trackKeyEvent(ke);
 }
 
 void Cheats::trackChatCommand(const std::string& cmd)
 {
-    if(!canCheatModeBeOn())
-        return;
-
-    cheatCmdTracker_->trackChatCommand(cmd);
+    if(areCheatsAllowed())
+        cheatCmdTracker_.trackChatCommand(cmd);
 }
 
 void Cheats::toggleCheatMode()
 {
-    if(!canCheatModeBeOn())
-        return;
-
-    isCheatModeOn_ = !isCheatModeOn_;
+    if(areCheatsAllowed())
+        isCheatModeOn_ = !isCheatModeOn_;
 }
 
 void Cheats::toggleHumanAIPlayer()
 {
-    if(!isCheatModeOn())
-        return;
-
-    if(GAMECLIENT.IsReplayModeOn())
-        return;
-
-    GAMECLIENT.ToggleHumanAIPlayer(AI::Info{AI::Type::Default, AI::Level::Easy});
+    if(isCheatModeOn() && !GAMECLIENT.IsReplayModeOn())
+        GAMECLIENT.ToggleHumanAIPlayer(AI::Info{AI::Type::Default, AI::Level::Easy});
 }
 
 void Cheats::armageddon()
 {
-    if(!isCheatModeOn())
-        return;
-
-    GAMECLIENT.CheatArmageddon();
+    if(isCheatModeOn())
+        GAMECLIENT.CheatArmageddon();
 }
 
-bool Cheats::canCheatModeBeOn() const
+bool Cheats::areCheatsAllowed() const
 {
     return world_.IsSinglePlayer();
 }

libs/s25main/Cheats.h

@@ -4,10 +4,10 @@
 
 #pragma once
 
+#include "CheatCommandTracker.h"
 #include <memory>
 #include <string>
 
-class CheatCommandTracker;
 class GameWorldBase;
 struct KeyEvent;
 
@@ -27,9 +27,9 @@ class Cheats
     void armageddon();
 
 private:
-    bool canCheatModeBeOn() const;
+    bool areCheatsAllowed() const;
 
-    std::unique_ptr<CheatCommandTracker> cheatCmdTracker_;
+    CheatCommandTracker cheatCmdTracker_;
     bool isCheatModeOn_ = false;
     GameWorldBase& world_;
 };

tests/s25Main/integration/testCheats.cpp

@@ -12,7 +12,7 @@ namespace {
 template<unsigned T_numPlayers>
 struct CheatWorldFixture : WorldFixture<CreateEmptyWorld, T_numPlayers>
 {
-    Cheats& cheats = world.GetCheats();
+    Cheats& cheats = this->world.GetCheats();
 };
 
 using CheatWorldFixture1P = CheatWorldFixture<1>;