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

Rework aircraft rally point handling. #16931

Merged
merged 1 commit into from Aug 15, 2019

Conversation

@pchote
Copy link
Member

commented Aug 11, 2019

Fixes #16912 and hopefully makes the code a bit more sensible and understandable.

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

@pchote pchote requested review from reaperrr and tovl Aug 11, 2019

self.QueueActivity(new TakeOff(self));
{
if (rp != null)
self.QueueActivity(new TakeOff(self, Target.FromCell(self.World, rp.Location)));

This comment has been minimized.

Copy link
@tovl

tovl Aug 11, 2019

Contributor

Why call TakeOff here but move.MoveTo in Resupply?
If you call move.MoveTo here as well (optionally wrapped in AttackMoveActivity if that is indeed the desired behaviour) then you can get rid of the target handling in TakeOff completely.

This comment has been minimized.

Copy link
@pchote

pchote Aug 11, 2019

Author Member

That was originally my plan, but then I realized that TakeOff with a target is not equivalent to move.MoveTo when there are activities queued afterwards. Trying to untangle and rewrite the different cases vastly increases the scope and regression potential of the PR.

The move.MoveTo in Resupply is copied directly from the Mobile branch, with the goal of having similar behaviour for all units.

@pchote pchote force-pushed the pchote:aircraft-rallypoints branch from b08f987 to ba4b894 Aug 11, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2019

Fixed and rebased.

@matjaeck
Copy link
Contributor

left a comment

LGTM.

@pchote pchote added the PR: Needs +2 label Aug 11, 2019

@reaperrr reaperrr merged commit 78302ea into OpenRA:bleed Aug 15, 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:aircraft-rallypoints 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
5 participants
You can’t perform that action at this time.