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

Remove some legacy activity cruft from Aircraft #16206

Merged
merged 2 commits into from Mar 9, 2019

Conversation

@reaperrr
Copy link
Contributor

commented Feb 15, 2019

You only need to look at the RTB activities to tell that Aircraft doing its own stuff here was somewhat redundant and just made things worse regarding debugging and code consistency.

Fixed some inconsistencies with the 'targetActor not available' case as well.

This is just a bit of clean-up before the upcoming larger refactor(s), so it would be nice to get this merged quickly.

@tovl

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Maybe this could be simplified further, considering that ReturnToBase and HeliReturnToBase already check if the targetActor is reserved?

@reaperrr reaperrr force-pushed the reaperrr:aircraft-rtb-cleanup branch from f59b453 to e4da27c Feb 16, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2019

You're right, the RTB activities should be able to handle that case on their own. Updated.

I also moved the SetLine to the activities, after the ChooseResupplier stuff, because that way the line is only set when the order actually succeeds and it simplifies the Aircraft code even further.

Furthermore, added a commit that makes HeliRTB use the ChooseResupplier from RTB.

@tovl
Copy link
Contributor

left a comment

Alright, looks good. Greatly improves DRYness. Also checked in game and as far as I can see this introduces no regressions. I don't know if my opinion counts for anything, but I think this can be merged.

@reaperrr reaperrr force-pushed the reaperrr:aircraft-rtb-cleanup branch from e4da27c to 5b9a741 Feb 24, 2019

reaperrr added 2 commits Feb 15, 2019
Remove some legacy activity cruft from Aircraft
You only need to look at the RTB activities to tell that
Aircraft doing its own stuff here was somewhat redundant
and just made things worse regarding debugging and
code consistency.
Make HeliReturnToBase use RTB.ChooseResupplier
Small consistency fix and prep for merging HRTB into RTB.

@reaperrr reaperrr force-pushed the reaperrr:aircraft-rtb-cleanup branch from 5b9a741 to 2071007 Feb 24, 2019

@reaperrr reaperrr requested a review from pchote Feb 24, 2019

@pchote pchote added this to the Next + 1 milestone Feb 24, 2019

updated

@pchote
pchote approved these changes Mar 9, 2019

@pchote pchote merged commit a7702a8 into OpenRA:bleed Mar 9, 2019

2 checks passed

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

@reaperrr reaperrr deleted the reaperrr:aircraft-rtb-cleanup branch Apr 4, 2019

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.