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

GroundState performance improvement #13603

Merged
merged 1 commit into from Aug 25, 2017

Conversation

Projects
None yet
5 participants
@forcecore
Contributor

forcecore commented Jul 8, 2017

Performance improvement for AI controlled Squads.

owner.Bot.QueueOrder(new Order("Attack", a, false) { TargetActor = owner.Bot.FindClosestEnemy(a.CenterPosition) });

This piece of code called FindClosestEnemy FOR EACH MEMBER of the squad, instead of only once. Some other code refactors for FindClosestEnemy code, too.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Jul 15, 2017

Member

I'd also be nice if you could squash the commits. :)

Member

abcdefg30 commented Jul 15, 2017

I'd also be nice if you could squash the commits. :)

@abcdefg30

Looks good to me otherwise. 👍

@RoosterDragon

This comment has been minimized.

Show comment
Hide comment
@RoosterDragon

RoosterDragon Jul 17, 2017

Member

For clarity, I'd still like to see #13603 (comment) so that we avoid the search cost if all the units are already busy.

Member

RoosterDragon commented Jul 17, 2017

For clarity, I'd still like to see #13603 (comment) so that we avoid the search cost if all the units are already busy.

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Jul 19, 2017

Contributor

FindClosestTarget is now run only when needed.

Contributor

forcecore commented Jul 19, 2017

FindClosestTarget is now run only when needed.

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Jul 19, 2017

Contributor

I ended up removing the "switch target" code. The current implementation of FindClosestTarget has nothing do do with idle unit's position and it doesn't make much sense to do retargeting for the idle units.

Contributor

forcecore commented Jul 19, 2017

I ended up removing the "switch target" code. The current implementation of FindClosestTarget has nothing do do with idle unit's position and it doesn't make much sense to do retargeting for the idle units.

@@ -71,7 +78,7 @@ public void Tick(Squad owner)
if (!owner.IsTargetValid)
{
var closestEnemy = owner.Bot.FindClosestEnemy(owner.Units.Random(owner.Random).CenterPosition);
var closestEnemy = FindClosestEnemy(owner);

This comment has been minimized.

@pchote

pchote Aug 17, 2017

Member

Note that this changes .Random to .First which is a behaviour change, not just a refactor/perf fix.

I'm hopeful that this is a good thing in that it might fix the bug where an AI squad will oscillate between two targets without ever moving to attack either. This will require some careful testing to qualify.

@pchote

pchote Aug 17, 2017

Member

Note that this changes .Random to .First which is a behaviour change, not just a refactor/perf fix.

I'm hopeful that this is a good thing in that it might fix the bug where an AI squad will oscillate between two targets without ever moving to attack either. This will require some careful testing to qualify.

@@ -130,7 +139,7 @@ public void Tick(Squad owner)
if (!owner.IsTargetValid)
{
var closestEnemy = owner.Bot.FindClosestEnemy(owner.Units.Random(owner.Random).CenterPosition);
var closestEnemy = FindClosestEnemy(owner);

This comment has been minimized.

@pchote

pchote Aug 17, 2017

Member

Likewise here.

@pchote

pchote Aug 17, 2017

Member

Likewise here.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Aug 17, 2017

Member

Needs a rebase now, sorry.

Member

pchote commented Aug 17, 2017

Needs a rebase now, sorry.

Disabled Ground squads finding targets individually
* Now they will tend to focus fire.
* Provides performance when when the AI designer implements better
  (and complex) target selection code
@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Aug 18, 2017

Contributor

Rebased. Hopefully, FindClosestTarget can be improved later with cloaking or Anti-air awareness.

Contributor

forcecore commented Aug 18, 2017

Rebased. Hopefully, FindClosestTarget can be improved later with cloaking or Anti-air awareness.

@forcecore forcecore referenced this pull request Aug 18, 2017

Merged

Navy separation #13605

@pchote pchote removed the PR: Rebase me! label Aug 18, 2017

@reaperrr reaperrr merged commit 739d0c0 into OpenRA:bleed Aug 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@forcecore forcecore deleted the forcecore:squadPerf branch Jan 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment