Fix Issue #35 "Improve mineral saturation" #50

ImpulseCloud · 2021-03-02T06:03:03Z

This will send new/idle workers to a non-saturated townhall. Townhalls will build an extra worker over Ideal and it will go to a new non-saturated townhall upon creation, so each townhall will continually build workers if there are any townhalls that are not yet saturated (until 80 total). After constructing a building, workers will go to an unsaturated townhall instead of the closest/starting one.

ImpulseCloud · 2021-03-02T06:05:28Z

I'm intending to do a proper evenly-redistribute-workers-among-bases when doing the decide-when-to-automatically-expand logic. Should I make a separate Issue for that, or include it with this one tomorrow?

alkurbatov · 2021-03-02T11:07:26Z

It is ok to continue work in the same issue.

src/core/Helpers.cpp

src/Hub.cpp

src/plugins/Miner.cpp

src/Hub.cpp

alkurbatov · 2021-03-02T11:51:19Z

Linter has failed:

src/Hub.cpp:257:  Lines should be <= 90 characters long  [whitespace/line_length] [2]
src/Hub.cpp:258:  Lines should be <= 90 characters long  [whitespace/line_length] [2]
src/Hub.cpp:261:  Lines should be <= 90 characters long  [whitespace/line_length] [2]
Done processing src/Hub.cpp
src/Hub.h:145:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

ImpulseCloud · 2021-03-04T05:08:23Z

Extended Expansion class and polished some places. Works pretty well now. I changed the MarinePush strategy a bit to reflect that it can't use Stim or Reactors correctly yet.

alkurbatov · 2021-03-04T07:58:56Z

Linter errors:

src/Hub.cpp:126:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:140:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:273:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:277:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:281:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:284:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:291:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:339:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:348:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:356:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.cpp:362:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/Hub.h:157:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/blueprints/TownHall.cpp:16:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/core/Map.cpp:98:  Missing space before {  [whitespace/braces] [5]
src/core/Map.cpp:155:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/plugins/Miner.cpp:26:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

src/Hub.cpp

src/blueprints/TownHall.cpp

src/core/Map.h

src/plugins/Miner.cpp

src/Hub.h

src/Hub.cpp

src/plugins/Plugin.h

src/Hub.cpp

src/plugins/Miner.cpp

src/Hub.cpp

ImpulseCloud · 2021-03-06T05:23:31Z

I re-wrote a couple sections. Works really well to keep pumping workers on 3 base.

Thank you for taking the time to review and comment.

alkurbatov

Looks much better, thank you!
Please, take a look at the remaining issues. This is not critical changes, but could be better to do them now.

src/plugins/Miner.cpp

src/core/Map.cpp

src/Hub.cpp

ImpulseCloud · 2021-03-10T15:36:49Z

I believe I have made requested changes now. Two of the Expansion/Owner questions I believe are best resolved within issue #53, "Secure/Defend new expansions" when there will be military actions to enable testing.

ImpulseCloud · 2021-03-10T20:18:41Z

Can this pull request be accepted now?

alkurbatov · 2021-03-11T13:58:56Z

I need to review it and since the request is relatively large it'll take some time.

alkurbatov

Several minor fixes left.

src/Hub.cpp

src/plugins/Miner.cpp

This will send new/idle workers to a non-saturated townhall. Townhalls will build an extra worker over Ideal and it will go to a new non-saturated townhall upon creation, so each townhall will continually build workers if there are any townhalls that are not yet saturated (until 80 total). After constructing a building, workers will go to an unsaturated townhall instead of the closest/starting one.

ImpulseCloud · 2021-03-12T15:57:30Z

Question: Should I have been just adding normal commits to resolve change-requests, and only doing a squash+force-push at the end after it's been reviewed successfully? I think perhaps the constant force-pushing is making looking at past change-requests more difficult.

alkurbatov · 2021-03-12T17:28:12Z

There are different ways of doing it. I prefer clean history without intermediate commits, however, you could have split the whole patch into several commits according to particular idea. To me it is ok in both ways.

alkurbatov · 2021-03-13T07:21:36Z

src/Dispatcher.cpp

@@ -145,6 +145,16 @@ void Dispatcher::OnUpgradeCompleted(sc2::UpgradeID id_) {
        i->OnUpgradeCompleted(id_);
 }

+void Dispatcher::OnUnitEnterVision(const sc2::Unit* unit_) {
+//    gHistory.info() << sc2::UnitTypeToName(unit_->unit_type) <<


Why this part was commented out?

It's triggered too often during big fights and slows the game. It's not just first time seen, but every time. If thought to leave it in commented out, so authors can easily uncomment it of they need to use for debugging, but maybe that isn't needed.

I think better to remove it, but ok.

alkurbatov · 2021-03-13T07:24:21Z

src/core/Helpers.cpp

@@ -12,14 +12,14 @@ IsUnit::IsUnit(sc2::UNIT_TYPEID type_): m_type(type_) {
 }

 bool IsUnit::operator()(const sc2::Unit& unit_) const {
-    return unit_.unit_type == m_type && unit_.build_progress >= 1.0f;
+    return unit_.unit_type == m_type && unit_.build_progress >= BUILD_FINISHED;


Seems that we need to add it to the api e.g. unit_.isBuildFinished().

.

alkurbatov · 2021-03-13T17:34:44Z

Very good job! Thank you!