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

Clear LastSearchFailed when starting a new FindAndDeliverResources activity #16983

Merged
merged 2 commits into from Aug 25, 2019

Conversation

@pchote
Copy link
Member

commented Aug 24, 2019

Fixes #16982.

@pchote pchote added this to the Next Release milestone Aug 24, 2019

@pchote pchote requested a review from tovl Aug 24, 2019

@pchote pchote force-pushed the pchote:fix-harvester-unload branch 2 times, most recently from dd51e5a to a8419c1 Aug 24, 2019

@tovl

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2019

This looks good. However, it would probably be cleaner if LastSearchFailed was a field of FindAndDeliverResources instead of Harvester. There is only one reference to LastSearchFailed outside of FindAndDeliverResources: in HarvesterBotModule.

As an aside, that particular line in HarvesterBotModule looks a bit fishy anyway. I'm not sure if Nextactivity is at all relevant here.

@pchote pchote force-pushed the pchote:fix-harvester-unload branch from a8419c1 to a552fc4 Aug 25, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2019

Good point. Updated.

On closer inspection that NextActivity check was preventing HarvesterBotModule from ever rescuing harvesters stuck in the FindAndDeliverResources.Wait state, so I removed that in addition to moving the flag.

A simple testcase to verify the fix is to create a map with a small resource patch (with no tree/mine) near the AI base, and a second patch far away. On bleed the harvester will stop outside the refinery after harvesting the first patch. Now they will move to the next patch.

@tovl
tovl approved these changes Aug 25, 2019
Copy link
Contributor

left a comment

👍

@reaperrr reaperrr merged commit 2b4ad71 into OpenRA:bleed Aug 25, 2019

2 checks passed

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

@pchote pchote deleted the pchote:fix-harvester-unload branch Aug 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.