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

Fix harvesters losing their last harvesting position when carried by carryall. #16778

Merged
merged 3 commits into from Jul 19, 2019

Conversation

@tovl
Copy link
Contributor

commented Jul 15, 2019

Fixes #16775

This is just a quick fix for next release. A proper fix would be the same as #16721 (comment) which would also conserve activities queued afterwards.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2019

Oh, so I was correct with my proposals? Good to know.

@pchote pchote added this to the Next Release milestone Jul 15, 2019

@pchote

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

Testing this shows another fairly bad regression with carryalls:

  1. Have a harvester and carryall working in normal automatic mode
  2. Wait for the harvester to request a pickup, then quickly issue a harvest order to a different field

Things get weird, depending on the exact timing. The most common case is that the carryall will deliver the harvester to the first location, even though it was cancelled well before the harvester was actually picked up. I had one time where the carryall was locked in a single facing directly above the harvester, sliding around the map while the harvester kept driving (it picked it up and delivered it to the wrong place when the harvester was stopped). Another time had the carryall pick up the harvester sideways.

@tovl tovl force-pushed the tovl:autocarryall-fix branch from a859cf3 to fd542db Jul 17, 2019

@tovl tovl force-pushed the tovl:autocarryall-fix branch from fd542db to 15d4f5d Jul 17, 2019

@tovl tovl force-pushed the tovl:autocarryall-fix branch from 15d4f5d to 456b615 Jul 17, 2019

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

The harvester sliding behind the harvester seems to be the WaitFor activity getting cancelled or never queued while the carryall still thinks it is locked. Somehow the activity queues get mangled but it turned out to be incredibly difficult to pinpoint exactly where things went wrong.

It is clear though that the root cause is that the carryall should not be messing with the activity queues at all, so I decided to just implement #16721 (comment) right now and get rid of all that.

The problem of the carryall delivering the harvester to the first location despite a new order being given seems to not be an actual regression but an old bug. This is fixed by introducing a new parent activity which can pass the destination to DeliverUnit at the last moment..

@pchote
pchote approved these changes Jul 19, 2019

@pchote pchote added the PR: Needs +2 label Jul 19, 2019

@teinarss teinarss merged commit 0e62490 into OpenRA:bleed Jul 19, 2019

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
Projects
None yet
4 participants
You can’t perform that action at this time.