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

Possible fix for #14102: Consider airfield available if already reserved for the same actor #16055

Merged
merged 1 commit into from Feb 15, 2019

Conversation

@portablestew
Copy link
Contributor

commented Jan 12, 2019

ReturnToBase.cs/Reservable.cs
Support checking if the Reservable is available to the actor, instead of reserved at all. This prevents a situation where two aircraft could reserve two airfields, but then desire to swap them (possibly the opposite airfields are closer when return-to-base is reissued).

I encounted a pair of yaks unable to land on the two airfields in allies-06b. They were stuck in ReturnToBase, unable to reserve an airfield because they were both already reserved by each other. I have not been able to reproduce this issue since making this change.

This AI behavior seems similar to the issue desribed in #14102.

@pchote
Copy link
Member

left a comment

I haven't tested this yet, but the code changes look reasonable. Just one naming nit to fix:

@@ -65,6 +65,12 @@ public static bool IsReserved(Actor a)
return res != null && res.reservedForAircraft != null && !res.reservedForAircraft.MayYieldReservation;
}

public static bool IsAvailableFor(Actor reservable, Actor forUser)

This comment has been minimized.

Copy link
@pchote

pchote Jan 27, 2019

Member

Can you please change this to IsAvailableFor(Actor self, Actor forAircraft) to match the the methods above?

This comment has been minimized.

Copy link
@portablestew

portablestew Jan 31, 2019

Author Contributor

I renamed it to forActor, since it's the Actor not the Aircraft (still consistent with the Reserve method). I also duplicated the ReturnToBase change to HeliReturnToBase.

@portablestew portablestew force-pushed the portablestew:returnToBaseReserved branch from 97a03fe to 369bed7 Jan 31, 2019

@portablestew portablestew force-pushed the portablestew:returnToBaseReserved branch from 369bed7 to d4d3880 Feb 11, 2019

@reaperrr reaperrr force-pushed the portablestew:returnToBaseReserved branch from d4d3880 to a31fa2c Feb 15, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Code looks good and would make sense even if it didn't fix #14102.
Tested and works as expected in-game.

I also sneaked in a small change of my own, namely using the new IsAvailableFor instead of IsReserved for the OrderTargeter and Enter/Repair order resolve in Aircraft.
Showing the crossed-out Enter cursor and rejecting Enter orders even when the building is reserved for the selected actor was a bit unintuitive.

I consider @pchote's comment a +1 on the code, since this works fine according to my testing and I'm working on a larger refactoring of the reserving logic locally, I'll go ahead and merge this now.

@reaperrr reaperrr merged commit a49287c into OpenRA:bleed Feb 15, 2019

2 checks passed

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

@portablestew portablestew deleted the portablestew:returnToBaseReserved branch Feb 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.