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

Add TurnToDock to Aircraft #15724

Merged
merged 5 commits into from Nov 3, 2018

Conversation

Projects
None yet
3 participants
@reaperrr
Copy link
Contributor

reaperrr commented Oct 21, 2018

There is no way to disable the "turn before landing on helipad/service depot" behavior of VTOLs on bleed and - as discussed on IRC - a separate boolean is better than using TurnToLand for this.

Additionally, pulled in some style changes from a later, larger aircraft activity refactor (see https://github.com/reaperrr/OpenRA/tree/merge-air-activities-WIP if you're interested, and don't get scared, it's not as bad as it looks. Probably.).

While I was at it, I also reduced the RA helipad reload animation speed to match the helipad in TD Split to #15725.

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch 2 times, most recently from bf2d601 to cc558ee Oct 23, 2018

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch from cc558ee to 52367e0 Nov 1, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 1, 2018

Updated, also added a small update rule that at least tells modders which actors might need manual fixing.

@abcdefg30
Copy link
Member

abcdefg30 left a comment

Looks good otherwise (untested).

Show resolved Hide resolved OpenRA.Mods.Common/Activities/Air/HeliReturnToBase.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Activities/Air/HeliReturnToBase.cs
Show resolved Hide resolved OpenRA.Mods.Common/UpdateRules/Rules/20180923/FixedHeliRTBTurnToLand.cs Outdated
if (turnToLandNode == null)
{
locations.Add("{0} ({1})".F(actorNode.Key, actorNode.Location.Filename));
yield break;

This comment has been minimized.

@abcdefg30

abcdefg30 Nov 1, 2018

Member

Does it make sense to break here? (If the actor has multiple aircraft traits, don't we want to report them all?)

This comment has been minimized.

@reaperrr

reaperrr Nov 1, 2018

Contributor

We only report the actor and file anyway, so this should be fine (and I think we don't really support multiple Aircraft traits yet, I just used a foreach to be on the safe side and because @pchote said he'd generally prefer ChildrenMatching over LastChildMatching for updating traits).

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch from 52367e0 to 7e92d57 Nov 1, 2018

@pchote
Copy link
Member

pchote left a comment

The Chinook in TD still turns to land, despite not defining TurnToLand: true. This suggests that something is still wrong with the code? Also needs a rebase.

Show resolved Hide resolved mods/cnc/rules/aircraft.yaml Outdated

reaperrr added some commits Oct 17, 2018

Move CalculateTurnRadius up in ReturnToBase
Just a slight readability improvement.
Remove separate AircraftInfo caching from ReturnToBase
This extra info caching was overkill and most likely had zero effect on performance.
Make HeliReturnToBase use a landingProcedures list
Like ReturnToBase already does. Makes them easier to compare and later merge.

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch 2 times, most recently from 860f1fa to 19a449c Nov 3, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 3, 2018

Updated and rebased.

The Chinook in TD still turns to land, despite not defining TurnToLand: true. This suggests that something is still wrong with the code? Also needs a rebase.

Yeah, a typical 'gotcha' resulting from Aircraft doing its own thing (see updated commit
8f6d17a ). Just reinforces that we need that activity refactor.

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch from 19a449c to fe5307f Nov 3, 2018

@reaperrr reaperrr removed the PR: Rebase me! label Nov 3, 2018

reaperrr added some commits Oct 31, 2018

Add TurnToDock to Aircraft
Instead of hard-coding a turn before VTOLs
land/dock on resupplier.

@reaperrr reaperrr force-pushed the reaperrr:fix-TurnToLand branch from fe5307f to 4677e41 Nov 3, 2018

@reaperrr reaperrr changed the title Fix VTOLs ignoring TurnToLand when returning to reload/repair Add TurnToDock to Aircraft Nov 3, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 3, 2018

Updated as discussed on IRC.

@pchote

pchote approved these changes Nov 3, 2018

@pchote pchote added the PR: Needs +2 label Nov 3, 2018

@abcdefg30 abcdefg30 merged commit 71fb670 into OpenRA:bleed Nov 3, 2018

2 checks passed

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

This comment has been minimized.

Copy link
Member

abcdefg30 commented Nov 3, 2018

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