From d1f78db6eca5a00867089b499cf91a19c83d0e47 Mon Sep 17 00:00:00 2001 From: Matthias Schwarzott Date: Fri, 29 Mar 2024 10:55:04 +0100 Subject: [PATCH 1/3] add Unittest for action_built --- CMakeLists.txt | 1 + src/action/action_built.cpp | 6 +- src/include/action/action_built.h | 4 +- tests/stratagus/test_action_built.cpp | 251 ++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 tests/stratagus/test_action_built.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6cbe8d1303..529013ca84 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -532,6 +532,7 @@ set(stratagus_generic_HDRS set(stratagus_tests_SRCS tests/main.cpp + tests/stratagus/test_action_built.cpp tests/stratagus/test_depend.cpp tests/stratagus/test_luacallback.cpp tests/stratagus/test_missile_fire.cpp diff --git a/src/action/action_built.cpp b/src/action/action_built.cpp index e53942aeb0..d25df00600 100644 --- a/src/action/action_built.cpp +++ b/src/action/action_built.cpp @@ -323,6 +323,10 @@ void COrder_Built::AiUnitKilled(CUnit &unit) static std::size_t FindCFramePercent(const std::vector &cframes, int percent) { + if (cframes.empty()) + { + return -1; + } const auto it = ranges::find_if(cframes, [&](const auto &frame) { return percent < frame.Percent; }); const std::size_t index = std::distance(cframes.begin(), it); @@ -340,7 +344,7 @@ void COrder_Built::UpdateConstructionFrame(CUnit &unit) const int percent = this->ProgressCounter / (type.Stats[unit.Player->Index].Costs[TimeCost] * 6); const auto index = FindCFramePercent(type.Construction->Frames, percent); - if (index != this->Frame) { + if (index != -1 && index != this->Frame) { this->Frame = index; const CConstructionFrame &cframe = type.Construction->Frames[index]; unit.Frame = (unit.Frame < 0) ? -cframe.Frame - 1 : cframe.Frame; diff --git a/src/include/action/action_built.h b/src/include/action/action_built.h index 996cfb8248..3be6e5d798 100644 --- a/src/include/action/action_built.h +++ b/src/include/action/action_built.h @@ -68,11 +68,11 @@ class COrder_Built : public COrder const CUnitPtr &GetWorker() const { return Worker; } CUnit *GetWorkerPtr() { return Worker; } -private: +protected: void Boost(CUnit &building, int amount, int varIndex) const; void UpdateConstructionFrame(CUnit &unit); -private: +protected: CUnitPtr Worker = nullptr; /// Worker building this unit int ProgressCounter = 0; /// Progress counter, in 1/100 cycles. bool IsCancelled = false; /// Cancel construction diff --git a/tests/stratagus/test_action_built.cpp b/tests/stratagus/test_action_built.cpp new file mode 100644 index 0000000000..a8a6b1e548 --- /dev/null +++ b/tests/stratagus/test_action_built.cpp @@ -0,0 +1,251 @@ +// _________ __ __ +// / _____// |_____________ _/ |______ ____ __ __ ______ +// \_____ \\ __\_ __ \__ \\ __\__ \ / ___\| | \/ ___/ +// / \| | | | \// __ \| | / __ \_/ /_/ > | /\___ | +// /_______ /|__| |__| (____ /__| (____ /\___ /|____//____ > +// \/ \/ \//_____/ \/ +// ______________________ ______________________ +// T H E W A R B E G I N S +// Stratagus - A free fantasy real time strategy game engine +// +/**@name test_action_built.cpp - The test file for action_built.cpp. */ +// +// (c) Copyright 2024 by Matthias Schwarzott +// +// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; only version 2 of the License. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA +// 02111-1307, USA. +// + +#include + +#include "stratagus.h" +#include "unit.h" +#include "action/action_built.h" +#include "construct.h" + +class tested_COrder_Built : public COrder_Built +{ +public: + using COrder_Built::ProgressCounter; + + int testUpdateConstrFrame_Percent(CUnit &unit, int percent, int initialFrame = 9999) + { + const int progressOnePercent = unit.Type->Stats[0].Costs[TimeCost] * 6; + return testUpdateConstrFrame_Progress(unit, percent * progressOnePercent, initialFrame); + } + + int testUpdateConstrFrame_Progress(CUnit &unit, int progress, int initialFrame = 9999) + { + unit.Frame = initialFrame; // to see if it was set + Frame = -5; // force re-calculation + ProgressCounter = progress; + UpdateConstructionFrame(unit); + return unit.Frame; + } +}; + +namespace { + void setFrames(std::vector &cframes, + const std::vector>& vec) + { + const std::size_t size = vec.size(); + cframes.resize(size); + for (int i = 0; i < size; ++i) { + cframes[i].Percent = vec[i].first; + cframes[i].Frame = vec[i].second; + } + } +}; + +TEST_CASE("COrder_Built Frame") +{ + CPlayer player; + player.Index = 0; + CUnitType type; + type.Stats[0].Costs[TimeCost] = 1; + CConstruction construction; + type.Construction = &construction; + auto &frames = construction.Frames; + CUnit unit; + unit.Type = &type; + unit.Player = &player; + tested_COrder_Built order; + + SUBCASE("init values") { + CHECK(unit.Frame == 0); + CHECK(order.ProgressCounter == 0); + } + + SUBCASE("size=0") { + setFrames(frames, {}); + + // check unit.Frame is not modified + CHECK(order.testUpdateConstrFrame_Percent(unit, 0, 123) == 123); + } + + SUBCASE("size=1") { + setFrames(frames, { + {0, 0}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 0) == 0); + CHECK(order.testUpdateConstrFrame_Percent(unit, 1) == 0); + CHECK(order.testUpdateConstrFrame_Percent(unit, 100) == 0); + } + + SUBCASE("large steps") { + setFrames(frames, { + { 0, 100}, + {50, 110}, + }); + + SUBCASE("normal") { + CHECK(order.testUpdateConstrFrame_Percent(unit, 0) == 100); + CHECK(order.testUpdateConstrFrame_Percent(unit, 1) == 100); + CHECK(order.testUpdateConstrFrame_Percent(unit, 49) == 100); + CHECK(order.testUpdateConstrFrame_Percent(unit, 50) == 110); + CHECK(order.testUpdateConstrFrame_Percent(unit, 51) == 110); + CHECK(order.testUpdateConstrFrame_Percent(unit, 100) == 110); + } + + SUBCASE("mirrored") { + CHECK(order.testUpdateConstrFrame_Percent(unit, 49, -1) == -101); + CHECK(order.testUpdateConstrFrame_Percent(unit, 50, -1) == -111); + } + + SUBCASE("out of bounds") { + // return frame of first entry + CHECK(order.testUpdateConstrFrame_Percent(unit, -1) == 100); + CHECK(order.testUpdateConstrFrame_Percent(unit, 101) == 110); + } + } + + SUBCASE("partially dense") + { + setFrames(frames, { + { 0, 0}, + { 1, 1}, + { 2, 2}, + { 65, 3}, + { 66, 4}, + { 67, 5}, + { 99, 6}, + {100, 7}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 0) == 0); + CHECK(order.testUpdateConstrFrame_Percent(unit, 1) == 1); + CHECK(order.testUpdateConstrFrame_Percent(unit, 2) == 2); + CHECK(order.testUpdateConstrFrame_Percent(unit, 3) == 2); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 50) == 2); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 64) == 2); + CHECK(order.testUpdateConstrFrame_Percent(unit, 65) == 3); + CHECK(order.testUpdateConstrFrame_Percent(unit, 66) == 4); + CHECK(order.testUpdateConstrFrame_Percent(unit, 67) == 5); + CHECK(order.testUpdateConstrFrame_Percent(unit, 68) == 5); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 98) == 5); + CHECK(order.testUpdateConstrFrame_Percent(unit, 99) == 6); + CHECK(order.testUpdateConstrFrame_Percent(unit, 100) == 7); + + SUBCASE("by progress") + { + CHECK(order.testUpdateConstrFrame_Progress(unit, 0) == 0); + CHECK(order.testUpdateConstrFrame_Progress(unit, 5) == 0); + CHECK(order.testUpdateConstrFrame_Progress(unit, 6) == 1); + CHECK(order.testUpdateConstrFrame_Progress(unit, 594) == 6); + CHECK(order.testUpdateConstrFrame_Progress(unit, 595) == 6); + CHECK(order.testUpdateConstrFrame_Progress(unit, 600) == 7); + } + } + + SUBCASE("repeated values") + { + setFrames(frames, { + { 0, 8}, + { 80, 1}, + { 90, 8}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 79) == 8); + CHECK(order.testUpdateConstrFrame_Percent(unit, 80) == 1); + CHECK(order.testUpdateConstrFrame_Percent(unit, 89) == 1); + CHECK(order.testUpdateConstrFrame_Percent(unit, 90) == 8); + } + + SUBCASE("first key larger") { + setFrames(frames, { + { 10, 5}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 9) == 5); + CHECK(order.testUpdateConstrFrame_Percent(unit, 10) == 5); + } + + SUBCASE("duplicate keys") + { + setFrames(frames, { + { 0, 0}, + { 20, 1}, + { 20, 2}, + { 60, 3}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 19) == 0); + CHECK(order.testUpdateConstrFrame_Percent(unit, 20) == 2); + CHECK(order.testUpdateConstrFrame_Percent(unit, 21) == 2); + } + + SUBCASE("wrong key order") { + setFrames(frames, { + { 75, 9}, + { 50, 8}, + { 0, 7}, + }); + + CHECK_FALSE(ranges::is_sorted(frames, std::less<>{}, &CConstructionFrame::Percent)); + const bool allgood = + order.testUpdateConstrFrame_Percent(unit, 0) == 7 && + order.testUpdateConstrFrame_Percent(unit, 49) == 7 && + order.testUpdateConstrFrame_Percent(unit, 50) == 8 && + order.testUpdateConstrFrame_Percent(unit, 74) == 8 && + order.testUpdateConstrFrame_Percent(unit, 75) == 9 && + order.testUpdateConstrFrame_Percent(unit, 76) == 9; + CHECK_FALSE(allgood); + } + + SUBCASE("too small key value") { + setFrames(frames, { + { -1, 7}, + {100, 8}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 0) == 7); + CHECK(order.testUpdateConstrFrame_Percent(unit, 1) == 7); + CHECK(order.testUpdateConstrFrame_Percent(unit, 99) == 7); + CHECK(order.testUpdateConstrFrame_Percent(unit, 100) == 8); + } + + SUBCASE("too large key value") { + setFrames(frames, { + { 0, 7}, + {101, 8}, + }); + + CHECK(order.testUpdateConstrFrame_Percent(unit, 0) == 7); + CHECK(order.testUpdateConstrFrame_Percent(unit, 100) == 7); + } +} From a0446161856a91f379dc3ec74a43c5fae7f14784 Mon Sep 17 00:00:00 2001 From: Matthias Schwarzott Date: Fri, 29 Mar 2024 22:59:05 +0100 Subject: [PATCH 2/3] Fix findings --- tests/stratagus/test_action_built.cpp | 28 ++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/stratagus/test_action_built.cpp b/tests/stratagus/test_action_built.cpp index a8a6b1e548..adb66f7928 100644 --- a/tests/stratagus/test_action_built.cpp +++ b/tests/stratagus/test_action_built.cpp @@ -43,7 +43,7 @@ class tested_COrder_Built : public COrder_Built { const int progressOnePercent = unit.Type->Stats[0].Costs[TimeCost] * 6; return testUpdateConstrFrame_Progress(unit, percent * progressOnePercent, initialFrame); - } + } int testUpdateConstrFrame_Progress(CUnit &unit, int progress, int initialFrame = 9999) { @@ -56,15 +56,17 @@ class tested_COrder_Built : public COrder_Built }; namespace { - void setFrames(std::vector &cframes, - const std::vector>& vec) + std::vector makeFrames( + const std::vector>& vec) { + std::vector cframes; const std::size_t size = vec.size(); cframes.resize(size); for (int i = 0; i < size; ++i) { cframes[i].Percent = vec[i].first; cframes[i].Frame = vec[i].second; } + return cframes; } }; @@ -88,14 +90,14 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("size=0") { - setFrames(frames, {}); + frames = makeFrames({}); // check unit.Frame is not modified CHECK(order.testUpdateConstrFrame_Percent(unit, 0, 123) == 123); } SUBCASE("size=1") { - setFrames(frames, { + frames = makeFrames({ {0, 0}, }); @@ -105,7 +107,7 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("large steps") { - setFrames(frames, { + frames = makeFrames({ { 0, 100}, {50, 110}, }); @@ -133,7 +135,7 @@ TEST_CASE("COrder_Built Frame") SUBCASE("partially dense") { - setFrames(frames, { + frames = makeFrames({ { 0, 0}, { 1, 1}, { 2, 2}, @@ -174,7 +176,7 @@ TEST_CASE("COrder_Built Frame") SUBCASE("repeated values") { - setFrames(frames, { + frames = makeFrames({ { 0, 8}, { 80, 1}, { 90, 8}, @@ -187,7 +189,7 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("first key larger") { - setFrames(frames, { + frames = makeFrames({ { 10, 5}, }); @@ -197,7 +199,7 @@ TEST_CASE("COrder_Built Frame") SUBCASE("duplicate keys") { - setFrames(frames, { + frames = makeFrames({ { 0, 0}, { 20, 1}, { 20, 2}, @@ -210,7 +212,7 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("wrong key order") { - setFrames(frames, { + frames = makeFrames({ { 75, 9}, { 50, 8}, { 0, 7}, @@ -228,7 +230,7 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("too small key value") { - setFrames(frames, { + frames = makeFrames({ { -1, 7}, {100, 8}, }); @@ -240,7 +242,7 @@ TEST_CASE("COrder_Built Frame") } SUBCASE("too large key value") { - setFrames(frames, { + frames = makeFrames({ { 0, 7}, {101, 8}, }); From 418576d498d60c1267c38a46f07bd59051c378ec Mon Sep 17 00:00:00 2001 From: Matthias Schwarzott Date: Fri, 29 Mar 2024 23:17:12 +0100 Subject: [PATCH 3/3] Use std::optional --- src/action/action_built.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/action/action_built.cpp b/src/action/action_built.cpp index d25df00600..da64a43a5d 100644 --- a/src/action/action_built.cpp +++ b/src/action/action_built.cpp @@ -321,11 +321,11 @@ void COrder_Built::AiUnitKilled(CUnit &unit) } -static std::size_t FindCFramePercent(const std::vector &cframes, int percent) +static std::optional FindCFramePercent(const std::vector &cframes, int percent) { if (cframes.empty()) { - return -1; + return {}; } const auto it = ranges::find_if(cframes, [&](const auto &frame) { return percent < frame.Percent; }); @@ -344,9 +344,9 @@ void COrder_Built::UpdateConstructionFrame(CUnit &unit) const int percent = this->ProgressCounter / (type.Stats[unit.Player->Index].Costs[TimeCost] * 6); const auto index = FindCFramePercent(type.Construction->Frames, percent); - if (index != -1 && index != this->Frame) { - this->Frame = index; - const CConstructionFrame &cframe = type.Construction->Frames[index]; + if (index.has_value() && index.value() != this->Frame) { + this->Frame = index.value(); + const CConstructionFrame &cframe = type.Construction->Frames[index.value()]; unit.Frame = (unit.Frame < 0) ? -cframe.Frame - 1 : cframe.Frame; } }