Reworks script aggregator. Cleans up API and code, and now handles scripts being removed from a set while we're iterating.
continuous-integration/drone/push Build is passing Details

Signed-off-by: Deukhoofd <Deukhoofd@gmail.com>
This commit is contained in:
Deukhoofd 2021-04-18 12:50:48 +02:00
parent 13df99a6cc
commit be10b3515c
Signed by: Deukhoofd
GPG Key ID: F63E044490819F6F
4 changed files with 83 additions and 80 deletions

View File

@ -9,73 +9,54 @@ namespace CreatureLib::Battling {
class ScriptAggregator { class ScriptAggregator {
const ScriptWrapper* _scripts; const ScriptWrapper* _scripts;
i32 _size; i32 _size;
i32 _index = 0; i32 _index = -1;
i32 _setIndex = 0; i32 _setIndex = -1;
inline void IncrementToNextNotNull(bool initialIncrement = true) { inline bool IncrementToNextNotNull() {
if (_scripts[_index].IsSet()) { if (_index != -1 && _scripts[_index].IsSet()) {
_setIndex++; _setIndex++;
if (_setIndex >= (i32)_scripts[_index].GetScriptSet()->Count()) { if (_setIndex >= (i32)_scripts[_index].GetScriptSet()->Count()) {
_setIndex = -1; _setIndex = -1;
} else { } else {
return; return true;
} }
} }
if (initialIncrement) for (_index++; _index < _size; ++_index) {
_index++;
while (HasNext()) {
if (_scripts[_index].HasValue()) { if (_scripts[_index].HasValue()) {
if (_scripts[_index].IsSet()) { if (_scripts[_index].IsSet()) {
_setIndex = -1; _setIndex = 0;
} }
return; return true;
} }
_index++;
} }
return false;
} }
public: public:
ScriptAggregator(){}; ScriptAggregator(){};
explicit ScriptAggregator(const ArbUt::List<ScriptWrapper>& scripts) explicit ScriptAggregator(const ArbUt::List<ScriptWrapper>& scripts)
: _scripts(scripts.RawData()), _size(scripts.Count()) { : _scripts(scripts.RawData()), _size(scripts.Count()){};
Reset();
};
inline void Reset() { inline void Reset() {
_index = 0; _index = -1;
if (_size > 0) { _setIndex = -1;
if (_scripts[_index].IsSet()) {
_setIndex = -1;
}
IncrementToNextNotNull(false);
}
} }
inline bool HasNext() { return _index < _size; } bool GetNext(ArbUt::BorrowedPtr<BattleScript>& out) {
if (!IncrementToNextNotNull()) {
std::optional<ArbUt::BorrowedPtr<BattleScript>> GetNextNotNull() { return false;
while (HasNext()) {
auto s = GetNext();
return s;
} }
return {};
}
ArbUt::BorrowedPtr<BattleScript> GetNext() {
auto& current = _scripts[_index]; auto& current = _scripts[_index];
if (!current.IsSet()) { if (!current.IsSet()) {
auto s = current.GetScript(); auto s = current.GetScript();
IncrementToNextNotNull(); out = *s;
return (*s); return true;
} else { } else {
auto& set = current.GetScriptSet()->GetIterator(); auto& set = current.GetScriptSet()->GetIterator();
if (_setIndex == -1) {
_setIndex = 0;
}
auto v = set[_setIndex]; auto v = set[_setIndex];
IncrementToNextNotNull(); out = v;
return v; return true;
} }
} }
}; };

View File

@ -2,8 +2,8 @@
{ \ { \
try { \ try { \
auto aggregator = source->GetScriptIterator(); \ auto aggregator = source->GetScriptIterator(); \
while (aggregator.HasNext()) { \ ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1; \
auto next = aggregator.GetNext(); \ while (aggregator.GetNext(next)) { \
try { \ try { \
next->hookName(__VA_ARGS__); \ next->hookName(__VA_ARGS__); \
} catch (const std::exception& e) { \ } catch (const std::exception& e) { \

View File

@ -27,8 +27,8 @@ TEST_CASE("Script Aggregator properly iterates containing script.") {
auto vec = ArbUt::List<ScriptWrapper>{ auto vec = ArbUt::List<ScriptWrapper>{
ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))}; ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNext(); while (aggr.GetNext(next)) {
next.As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 1); CHECK(ran == 1);
@ -44,8 +44,8 @@ TEST_CASE("Script Aggregator properly iterates multiple scripts.") {
ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script2)), ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script2)),
ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script3))}; ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script3))};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNext(); while (aggr.GetNext(next)) {
next.As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
@ -62,13 +62,33 @@ TEST_CASE("Script Aggregator properly iterates Script Set.") {
set.Add(script3); set.Add(script3);
auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)}; auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNextNotNull(); while (aggr.GetNext(next)) {
next.value().As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
} }
TEST_CASE("Script Aggregator properly iterates Script Set when removing from it.") {
BattleScript* script = new TestScript("test");
BattleScript* script2 = new TestScript("test2");
BattleScript* script3 = new TestScript("test3");
auto ran = 0;
auto set = ScriptSet();
set.Add(script);
set.Add(script2);
set.Add(script3);
auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
auto aggr = ScriptAggregator(vec);
ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
while (ran <= 2) {
aggr.GetNext(next);
next.As<TestScript>()->TestMethod(ran);
}
set.Remove("test3"_cnc);
REQUIRE_FALSE(aggr.GetNext(next));
}
TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.") { TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.") {
auto script = std::make_unique<TestScript>("test"); auto script = std::make_unique<TestScript>("test");
BattleScript* script2 = new TestScript("test2"); BattleScript* script2 = new TestScript("test2");
@ -81,9 +101,9 @@ TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.")
ScriptWrapper::FromSet(&set), ScriptWrapper::FromSet(&set),
ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))}; ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNextNotNull(); while (aggr.GetNext(next)) {
next.value().As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
} }
@ -100,9 +120,9 @@ TEST_CASE("Script Aggregator properly iterates data of Script and Script Set.")
ArbUt::List<ScriptWrapper>{ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script)), ArbUt::List<ScriptWrapper>{ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script)),
ScriptWrapper::FromSet(&set)}; ScriptWrapper::FromSet(&set)};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNextNotNull(); while (aggr.GetNext(next)) {
next.value().As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
} }
@ -121,9 +141,9 @@ TEST_CASE("Script Aggregator properly iterates data of Script, Script Set and Sc
ScriptWrapper::FromSet(&set), ScriptWrapper::FromSet(&set),
ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script4))}; ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script4))};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNextNotNull(); while (aggr.GetNext(next)) {
next.value().As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 4); CHECK(ran == 4);
} }
@ -143,17 +163,16 @@ TEST_CASE("Script Aggregator properly iterates multiple script sets.") {
}; };
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
auto ran = 0; auto ran = 0;
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
auto next = aggr.GetNextNotNull(); while (aggr.GetNext(next)) {
next.value().As<TestScript>()->TestMethod(ran); next.As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
aggr.Reset(); aggr.Reset();
ran = 0; ran = 0;
while (aggr.HasNext()) { while (aggr.GetNext(next)) {
auto next = aggr.GetNextNotNull(); next.As<TestScript>()->TestMethod(ran);
next.value().As<TestScript>()->TestMethod(ran);
} }
CHECK(ran == 3); CHECK(ran == 3);
} }
@ -162,9 +181,8 @@ TEST_CASE("Script Aggregator properly iterates when empty.") {
auto ran = 0; auto ran = 0;
auto vec = ArbUt::List<ScriptWrapper>{}; auto vec = ArbUt::List<ScriptWrapper>{};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
THROW("Aggregator returned a script, but should have been empty."); REQUIRE_FALSE(aggr.GetNext(next));
}
CHECK(ran == 0); CHECK(ran == 0);
} }
@ -174,7 +192,8 @@ TEST_CASE("Script Aggregator properly iterates empty Script Set.") {
auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)}; auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
auto aggr = ScriptAggregator(vec); auto aggr = ScriptAggregator(vec);
aggr.Reset(); aggr.Reset();
while (aggr.HasNext()) { ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
while (aggr.GetNext(next)) {
ran++; ran++;
} }
CHECK(ran == 0); CHECK(ran == 0);

View File

@ -43,30 +43,34 @@ protected:
TEST_CASE("Script source with unset script ptr.") { TEST_CASE("Script source with unset script ptr.") {
auto source = ScriptSourceWithScriptPtr(); auto source = ScriptSourceWithScriptPtr();
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
REQUIRE_FALSE(scripts.HasNext()); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE_FALSE(scripts.GetNext(next));
} }
TEST_CASE("Script source with script ptr being set.") { TEST_CASE("Script source with script ptr being set.") {
auto source = ScriptSourceWithScriptPtr(); auto source = ScriptSourceWithScriptPtr();
source.ScriptPtr = std::make_unique<TestScript>("foobar"); source.ScriptPtr = std::make_unique<TestScript>("foobar");
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
[[maybe_unused]] auto first = scripts.GetNext(); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE(scripts.GetNext(next));
} }
TEST_CASE("Script source with script ptr being set after first iteration.") { TEST_CASE("Script source with script ptr being set after first iteration.") {
auto source = ScriptSourceWithScriptPtr(); auto source = ScriptSourceWithScriptPtr();
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
REQUIRE_FALSE(scripts.HasNext()); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE_FALSE(scripts.GetNext(next));
source.ScriptPtr = std::make_unique<TestScript>("foobar"); source.ScriptPtr = std::make_unique<TestScript>("foobar");
scripts = source.GetScriptIterator(); scripts = source.GetScriptIterator();
[[maybe_unused]] auto first = scripts.GetNext(); REQUIRE(scripts.GetNext(next));
} }
TEST_CASE("Script source with empty script set.") { TEST_CASE("Script source with empty script set.") {
auto source = ScriptSourceWithScriptSet(); auto source = ScriptSourceWithScriptSet();
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
scripts.Reset(); scripts.Reset();
REQUIRE_FALSE(scripts.HasNext()); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE_FALSE(scripts.GetNext(next));
} }
TEST_CASE("Script source with single item script set.") { TEST_CASE("Script source with single item script set.") {
@ -74,9 +78,9 @@ TEST_CASE("Script source with single item script set.") {
auto s = new TestScript("foobar"); auto s = new TestScript("foobar");
source.Set.Add(s); source.Set.Add(s);
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
auto first = scripts.GetNextNotNull(); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE(first.has_value()); REQUIRE(scripts.GetNext(next));
CHECK(first.value()->GetName() == "foobar"); CHECK(next->GetName() == "foobar");
} }
TEST_CASE("Script source with multiple item script set.") { TEST_CASE("Script source with multiple item script set.") {
@ -86,12 +90,11 @@ TEST_CASE("Script source with multiple item script set.") {
source.Set.Add(s); source.Set.Add(s);
source.Set.Add(s2); source.Set.Add(s2);
auto scripts = source.GetScriptIterator(); auto scripts = source.GetScriptIterator();
auto first = scripts.GetNextNotNull(); ArbUt::BorrowedPtr<CreatureLib::Battling::BattleScript> next = (CreatureLib::Battling::BattleScript*)1;
REQUIRE(first.has_value()); REQUIRE(scripts.GetNext(next));
CHECK(first.value()->GetName() == "foobar"); CHECK(next->GetName() == "foobar");
auto second = scripts.GetNextNotNull(); REQUIRE(scripts.GetNext(next));
REQUIRE(second.has_value()); CHECK(next->GetName() == "foobar2");
CHECK(second.value()->GetName() == "foobar2");
} }
#endif #endif