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 Repairable crash #15822

Merged
merged 2 commits into from Nov 19, 2018

Conversation

Projects
None yet
2 participants
@reaperrr
Copy link
Contributor

reaperrr commented Nov 15, 2018

MoveAdjacentTo is a Mobile-only activity, and Repairable.AfterReachActivities was generally written with ground actors in mind.
Reusing the "Enter" logic in Aircraft is both simpler and safer (regarding regression risk) than trying to adapt Repairable.

Fixes #15820.
Closes #13975.

Fix Repairable crash
MoveAdjacentTo is a Mobile-only activity.

@reaperrr reaperrr added this to the Next Release milestone Nov 15, 2018

@pchote pchote removed the PR: For stable label Nov 17, 2018

@pchote
Copy link
Member

pchote left a comment

This indeed fixes the crash, but helicopters now repair in the air above the helipad/service depot without landing.

Repro case: build an apache/orca in TD, damage them, then use the repair order generator on them.

Fix aircraft being repaired mid-air
Repairable was originally written for ground actors,
so it's both safer and much easier to just handle this in Aircraft directly.
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Nov 19, 2018

BTW, when did we (accidentally?) fix the repair cursor for aircraft?

#13975 is still open, and I don't see a mention in the changelog.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 19, 2018

If one of my PRs fixed it, it was by accident.

@pchote

pchote approved these changes Nov 19, 2018

@pchote pchote merged commit b8d3c9f into OpenRA:bleed Nov 19, 2018

2 checks passed

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

This comment has been minimized.

Copy link
Member

pchote commented Nov 19, 2018

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/d94fc9a51c5b87dca38d8f6b0d356e01d17fc2c0

If one of my PRs fixed it, it was by accident.

it was fixed by #15543, which is now reflected in the changelog

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