From fd16152121f0449cd98e01a295c2556f3b61b8b5 Mon Sep 17 00:00:00 2001
From: Deukhoofd <deukhoofd@gmail.com>
Date: Tue, 12 Nov 2019 18:38:09 +0100
Subject: [PATCH] Make scriptset return scripts in order of insertion, unit
 tests for ScriptSource

---
 .../ScriptHandling/ScriptAggregator.hpp       |  19 ++--
 src/Battling/ScriptHandling/ScriptSet.hpp     |  23 ++--
 .../ScriptTests/ScriptSourceTest.cpp          | 104 ++++++++++++++++++
 3 files changed, 127 insertions(+), 19 deletions(-)
 create mode 100644 tests/BattleTests/ScriptTests/ScriptSourceTest.cpp

diff --git a/src/Battling/ScriptHandling/ScriptAggregator.hpp b/src/Battling/ScriptHandling/ScriptAggregator.hpp
index 2466600..8b195ec 100644
--- a/src/Battling/ScriptHandling/ScriptAggregator.hpp
+++ b/src/Battling/ScriptHandling/ScriptAggregator.hpp
@@ -13,26 +13,26 @@ namespace CreatureLib::Battling{
         std::vector<ScriptWrapper> _scripts;
         int _index = 0;
         bool _isSetSet = false;
-        std::__detail::_Node_const_iterator<std::pair<const std::string, Script *>, false, true> _setIterator;
-        std::__detail::_Node_const_iterator<std::pair<const std::string, Script *>, false, true> _setEnd;
+        const std::vector<Script*>* _setScripts;
+        int _setIndex;
     public:
         ScriptAggregator(std::vector<ScriptWrapper> scripts) : _scripts(std::move(scripts)){
         };
 
         bool HasNext(){
-            return _index < _scripts.size() || (_isSetSet && _setIterator != _setEnd);
+            return _index < _scripts.size() || (_isSetSet && _setIndex < _setScripts->size());
         }
 
         Script* GetNext(){
             // We can probably do this in a cleaner version once C++ 20 drops with Coroutine support.
             if (_isSetSet){
-                if (_setIterator == _setEnd){
+                if (_setIndex >= _setScripts->size()){
                     _isSetSet = false;
                     return GetNext();
                 }
-                auto s = _setIterator->second;
-                _setIterator.operator++();
-                if (_setIterator == _setEnd){
+                auto s = _setScripts->at(_setIndex);
+                _setIndex++;
+                if (_setIndex >= _setScripts->size()){
                     _isSetSet = false;
                 }
                 return s;
@@ -51,10 +51,9 @@ namespace CreatureLib::Battling{
                 auto set = next.GetScriptSet();
                 if (set->Count() == 0)
                     return GetNext();
-                auto it = set->GetIterator();
-                _setIterator = it->begin();
-                _setEnd = it->end();
+                _setScripts = set->GetIterator();
                 _isSetSet = true;
+                _setIndex = 0;
                 return GetNext();
             }
             throw NotReachableException();
diff --git a/src/Battling/ScriptHandling/ScriptSet.hpp b/src/Battling/ScriptHandling/ScriptSet.hpp
index ea39e43..78b537e 100644
--- a/src/Battling/ScriptHandling/ScriptSet.hpp
+++ b/src/Battling/ScriptHandling/ScriptSet.hpp
@@ -7,27 +7,32 @@
 
 namespace CreatureLib::Battling{
     class ScriptSet{
-        std::unordered_map<std::string, Script*> _scripts;
+        std::vector<Script*> _scripts;
+        std::unordered_map<std::string, size_t> _lookup;
     public:
         void Add(Script* script){
-            auto f = _scripts.find(script->GetName());
-            if (f != _scripts.end()){
-                f->second->Stack();
-            }
-            else{
-                _scripts.insert({script->GetName(), script});
+            auto f = _lookup.find(script->GetName());
+            if (f != _lookup.end()){
+                _scripts[f.operator*().second]->Stack();
+                return;
             }
+            _scripts.push_back(script);
+            _lookup.insert({script->GetName(), _scripts.size() - 1});
         }
 
         void Remove(const std::string& key){
-            _scripts.erase(key);
+            auto find = _lookup.find(key);
+            if (find != _lookup.end()){
+                _scripts.erase(_scripts.begin() + find.operator*().second);
+                _lookup.erase(key);
+            }
         }
 
         size_t Count() const{
             return _scripts.size();
         }
 
-        const std::unordered_map<std::string, Script *> * GetIterator() const{
+        const std::vector<Script *> * GetIterator() const{
             return &_scripts;
         }
     };
diff --git a/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp b/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp
new file mode 100644
index 0000000..3e685a8
--- /dev/null
+++ b/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp
@@ -0,0 +1,104 @@
+#ifdef TESTS_BUILD
+
+#include <utility>
+
+#include "../../../extern/catch.hpp"
+#include "../../../src/Battling/ScriptHandling/ScriptSource.hpp"
+#include "../../../src/Battling/ScriptHandling/ScriptAggregator.hpp"
+
+using namespace CreatureLib;
+using namespace CreatureLib::Battling;
+
+class TestScript : public Script{
+public:
+    explicit TestScript(std::string name) : Script(std::move(name)){};
+
+    void TestMethod(int& runCount)  {
+        runCount++;
+    }
+};
+
+class ScriptSourceWithScriptPtr : public ScriptSource{
+public:
+    Script* ScriptPtr = nullptr;
+protected:
+    void GetActiveScripts(std::vector<ScriptWrapper> &scripts) override {
+        scripts.emplace_back(&ScriptPtr);
+    }
+};
+
+class ScriptSourceWithScriptSet : public ScriptSource{
+public:
+    ScriptSet Set;
+protected:
+    void GetActiveScripts(std::vector<ScriptWrapper> &scripts) override {
+        scripts.emplace_back(&Set);
+    }
+};
+
+
+TEST_CASE( "Script source with unset script ptr.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptPtr();
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first == nullptr);
+}
+
+TEST_CASE( "Script source with script ptr being set.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptPtr();
+    source.ScriptPtr = new Script("foobar");
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first != nullptr);
+    delete source.ScriptPtr;
+}
+
+TEST_CASE( "Script source with script ptr being set after first iteration.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptPtr();
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first == nullptr);
+    source.ScriptPtr = new Script("foobar");
+    scripts = source.GetScriptIterator();
+    first = scripts.GetNext();
+    CHECK(first != nullptr);
+    delete source.ScriptPtr;
+}
+
+TEST_CASE( "Script source with empty script set.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptSet();
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first == nullptr);
+}
+
+TEST_CASE( "Script source with single item script set.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptSet();
+    auto s = new Script("foobar");
+    source.Set.Add(s);
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first != nullptr);
+    CHECK (first->GetName() == "foobar");
+    delete s;
+}
+
+TEST_CASE( "Script source with multiple item script set.", "[Battling, Scripting]" ) {
+    auto source = ScriptSourceWithScriptSet();
+    auto s = new Script("foobar");
+    auto s2 = new Script("foobar2");
+    source.Set.Add(s);
+    source.Set.Add(s2);
+    auto scripts = source.GetScriptIterator();
+    auto first = scripts.GetNext();
+    CHECK(first != nullptr);
+    CHECK (first->GetName() == "foobar");
+    auto second = scripts.GetNext();
+    CHECK(second != nullptr);
+    CHECK (second->GetName() == "foobar2");
+    delete s;
+}
+
+
+#endif
+