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.

Sign up for GitHub

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

Jump to bottom

Make ReturnToBase order queueable #16207

Merged
merged 2 commits into from Feb 22, 2019
Merged

Make ReturnToBase order queueable #16207

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7734b83
make ReturnToBase order queueable
tovl Feb 15, 2019
9d9ffe2
unreserve when not queued
tovl Feb 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions OpenRA.Mods.Common/Traits/Air/Aircraft.cs
View file
Open in desktop
Expand Up @@ -820,12 +820,13 @@ public void ResolveOrder(Actor self, Order order)
}
else if (order.OrderString == "ReturnToBase" && rearmableInfo != null && rearmableInfo.RearmActors.Any())
{
UnReserve();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be safer to keep this unconditional UnReserve, in case a queued RTB is cancelled before running.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line entirely seems to have been an oversight, but shouldn't it be conditional on not being queued? That is how it works for the other orders as well. I have added this in the new commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced here - IMO it doesn't make sense to hold the reservation and block other actors from using the airfield until the current activities are completed. Do we know why the other orders do it like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The queued unreserve was introduced in #8886. There is no explicit mention of the reservation, so it may not have been intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it may be because of the TakeOff order in UnReserve? Because it only checks the height at the moment the order is given, not when the queued order is executed this may lead to odd behaviour.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. That's a rather ugly hack.

Lets take this as it is then, and look into improving the UnReserve behaviour in a future PR.

self.CancelActivity();
if (!order.Queued)
UnReserve();

if (!Info.CanHover)
self.QueueActivity(new ReturnToBase(self, Info.AbortOnResupply, null, false));
self.QueueActivity(order.Queued, new ReturnToBase(self, Info.AbortOnResupply, null, false));
else
self.QueueActivity(new HeliReturnToBase(self, Info.AbortOnResupply, null, false));
self.QueueActivity(order.Queued, new HeliReturnToBase(self, Info.AbortOnResupply, null, false));
}
}

Expand Down