Skip to content
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

HackyAI: Refactor and remove duplicated logic. #14685

Merged
merged 1 commit into from Feb 21, 2018

Conversation

Projects
None yet
5 participants
@LipkeGu
Copy link
Member

LipkeGu commented Jan 7, 2018

This PR combines the duplicated logic of EnoughWaterToBuildNaval and CloseEnoughToWater in IsAreaAvailable<T>().

IsAreaAvailable<T>() takes the int radius and HashSet<string> terrainTypes arguments to be flexible with terrain types. So it is able to use it not only for water.

This PR also adds GetActorsWithTrait<T> and removes CountBuildings() and CountUnits() both does the same lookup and is different by the name of an string argument.

These changes was by the way done a long time ago by me in my AI-Overhaul branch and worked so far.
So i ripped them out of the branch.

@LipkeGu LipkeGu force-pushed the LipkeGu:hackyai_duplicated_logic branch 3 times, most recently from f7a1d1a to 8b5a5c2 Jan 7, 2018

@LipkeGu

This comment has been minimized.

Copy link
Member Author

LipkeGu commented Jan 7, 2018

updated

@ROCKNROLLKID

This comment has been minimized.

Copy link

ROCKNROLLKID commented Jan 8, 2018

Nice to see you back on this.

@LipkeGu LipkeGu force-pushed the LipkeGu:hackyai_duplicated_logic branch 2 times, most recently from 6e01dc4 to b00c117 Jan 9, 2018

@LipkeGu

This comment has been minimized.

Copy link
Member Author

LipkeGu commented Jan 9, 2018

The newly pushed commit allows InitializeBase() to use FindAndDeployMcv() so we can reuse it.
The commit also removes the foreach loop by selecting mcv from the list which are idle and takes the first it can get.


foreach (var b in baseProviders)
{
// TODO: Properly check building foundation rather than 3x3 area

This comment has been minimized.

@GraionDilach

GraionDilach Jan 13, 2018

Contributor

You need to keep/carry over this comment - this is about the shortcomings of relying on Util.AdjacentCells which you're still using.

This comment has been minimized.

@LipkeGu

LipkeGu Jan 13, 2018

Author Member

ok... the comment is carried over back

// backup location.
void FindAndDeployBackupMcv(Actor self)
// Find any MCV and deploy them at a sensible location.
Actor FindAndDeployMcv(Actor self, bool move = true)

This comment has been minimized.

@GraionDilach

GraionDilach Jan 13, 2018

Contributor

You never rely on the fallback default of move, so there's no point including it.

@LipkeGu LipkeGu force-pushed the LipkeGu:hackyai_duplicated_logic branch from b00c117 to 2a9e5d0 Jan 13, 2018

@LipkeGu

This comment has been minimized.

Copy link
Member Author

LipkeGu commented Jan 13, 2018

rebased and updated

@@ -323,7 +323,6 @@ ActorInfo ChooseBuildingToBuild(ProductionQueue queue)
HackyAI.BotDebug("{0} decided to build {1}: Priority override (is low power)", queue.Actor.Owner, power.Name);
else
HackyAI.BotDebug("{0} decided to build {1}: Priority override (would be low power)", queue.Actor.Owner, power.Name);

This comment has been minimized.

@reaperrr

reaperrr Jan 13, 2018

Contributor

Style nit: I'd prefer to keep this empty line, for readability.

@GraionDilach
Copy link
Contributor

GraionDilach left a comment

👍 after the formatting issue has be resolved.

@LipkeGu LipkeGu force-pushed the LipkeGu:hackyai_duplicated_logic branch from 2a9e5d0 to 40c15bf Jan 14, 2018

@LipkeGu

This comment has been minimized.

Copy link
Member Author

LipkeGu commented Jan 14, 2018

updated

var countOwnAir = CountUnits(actorInfo.Name, Player);
var countBuildings = aircraftInfo.RearmBuildings.Sum(b => CountBuilding(b, Player));
var countOwnAir = CountActorsWithTrait<IPositionable>(actorInfo.Name, Player);
var countBuildings = aircraftInfo.RearmBuildings.Sum(b => CountActorsWithTrait<Buildable>(b, Player));

This comment has been minimized.

@reaperrr

reaperrr Feb 21, 2018

Contributor

This should be Building instead of Buildable, but I'll fix it myself so we can merge this quickly.

@reaperrr reaperrr force-pushed the LipkeGu:hackyai_duplicated_logic branch from 40c15bf to cd9c35b Feb 21, 2018

@reaperrr reaperrr merged commit 21472f2 into OpenRA:bleed Feb 21, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.