-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Action_built: Add unittest for percentage lookup #647
Conversation
src/action/action_built.cpp
Outdated
@@ -323,6 +323,10 @@ void COrder_Built::AiUnitKilled(CUnit &unit) | |||
|
|||
static std::size_t FindCFramePercent(const std::vector<CConstructionFrame> &cframes, int percent) | |||
{ | |||
if (cframes.empty()) | |||
{ | |||
return -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning std::optional<std::size_t>
might be an option. that ensures the test for validity.
.editorconfig
Outdated
@@ -8,6 +8,7 @@ root = true | |||
charset = utf-8 | |||
end_of_line = lf | |||
insert_final_newline = true | |||
trim_trailing_whitespace = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to split PR next time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #648
{ | ||
const int progressOnePercent = unit.Type->Stats[0].Costs[TimeCost] * 6; | ||
return testUpdateConstrFrame_Progress(unit, percent * progressOnePercent, initialFrame); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not formatted correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
void setFrames(std::vector<CConstructionFrame> &cframes, | ||
const std::vector<std::pair<int, int>>& vec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would std::vector<CConstructionFrame> makeFrames(const std::vector<std::pair<int, int>>& vec)
make sense instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed. But does not look too different.
7b835dd
to
a044616
Compare
Testing the percentage lookup code fixed some days ago.
TODO: What else to test in here? and how?