Refactor Aircraft idle behavior #15873

reaperrr · 2018-12-02T13:49:35Z

A follow-up to #15848.

Aircraft can now define a list of idle actions to try to trigger when becoming idle.
Additionally, this is done via a 'priority list', so if the first entry is currently (example: Land if target cell is blocked) or generally (Hover if CanHover is false) not possible, the actor will attempt to trigger the 2nd action instead, and so on.
This approach makes life easier for modders, as an invalid entry doesn't necessarily break anything, instead it will simply be skipped.
This also allows for some nice code clean-ups, some as part of this PR, some later when the plane and heli activities are merged and generalized.

Depends on #15848.
Depedency for #15803.

pchote · 2018-12-04T20:25:33Z

Needs a rebase.

reaperrr · 2018-12-04T21:14:12Z

Rebased.

OpenRA.Mods.Common/Activities/Air/FlyOffMap.cs

OpenRA.Mods.Common/Traits/Air/Aircraft.cs

reaperrr · 2018-12-09T15:24:24Z

Updated. Left the changes in a separate commit for now.

OpenRA.Mods.Common/Traits/Air/Aircraft.cs

And remove all traits, properties and hard-codings that would conflict with it. A single list with possible actions is better than multiple, possibly conflicting booleans and traits. Designed to support fallbacks in case a listed action isn't supported by that particular actor, to make life a little easier for modders.

Ran update rules on them first and then cleaned up.

reaperrr · 2018-12-13T14:11:15Z

Updated.

abcdefg30

This has some gameplay regressions:

Yaks now always go to rearm when idling, even if you explicitly gave them a move order.
Yaks reserve the next airfield when ordered to attack. (You can test that by trying to build another one, it can't exit the airfield and is blocked.)

obrakmann · 2018-12-17T23:20:56Z

Yaks now always go to rearm when idling, even if you explicitly gave them a move order.

That's basically how it's supposed to work, yes. If we were to change that, the Fly activity would need a parameter to decide whether to go idle or to queue a FlyCircle when it's done.

Yaks reserve the next airfield when ordered to attack. (You can test that by trying to build another one, it can't exit the airfield and is blocked.)

I was not able to reproduce that.

abcdefg30 · 2018-12-18T10:24:01Z

That's basically how it's supposed to work, yes. If we were to change that, the Fly activity would need a parameter to decide whether to go idle or to queue a FlyCircle when it's done.

That makes Yaks unusable. 👎 to this PR if that is the intended behaviour.

obrakmann · 2018-12-18T12:45:04Z

Something like this on top of this PR would yield the behaviour where planes would circle if ordered to move to a location, but otherwise RTB when becoming idle, which could probably be a compromise.

AFAIK the idea is otherwise still that helicopters are allowed to stay on the battlefield while planes should RTB asap. Or has anything changed wrt that policy?

--- a/OpenRA.Mods.Common/Activities/Air/Fly.cs
+++ b/OpenRA.Mods.Common/Activities/Air/Fly.cs
@@ -23,15 +23,17 @@ public class Fly : Activity
                readonly WDist maxRange;
                readonly WDist minRange;
                bool soundPlayed;
+               bool loiter;
 
-               public Fly(Actor self, Target t)
+               public Fly(Actor self, Target t, bool loiter = false)
                {
                        aircraft = self.Trait<Aircraft>();
                        target = t;
+                       this.loiter = loiter;
                }
 
-               public Fly(Actor self, Target t, WDist minRange, WDist maxRange)
-                       : this(self, t)
+               public Fly(Actor self, Target t, WDist minRange, WDist maxRange, bool loiter = false)
+                       : this(self, t, loiter)
                {
                        this.maxRange = maxRange;
                        this.minRange = minRange;
@@ -79,14 +81,22 @@ public override Activity Tick(Actor self)
                        var insideMaxRange = maxRange.Length > 0 && target.IsInRange(aircraft.CenterPosition, maxRange);
                        var insideMinRange = minRange.Length > 0 && target.IsInRange(aircraft.CenterPosition, minRange);
                        if (insideMaxRange && !insideMinRange)
+                       {
+                               if (loiter && NextInQueue == null)
+                                       Queue(new FlyCircle(self));
                                return NextActivity;
+                       }
 
                        var d = target.CenterPosition - self.CenterPosition;
 
                        // The next move would overshoot, so consider it close enough
                        var move = aircraft.FlyStep(aircraft.Facing);
                        if (d.HorizontalLengthSquared < move.HorizontalLengthSquared)
+                       {
+                               if (loiter && NextInQueue == null)
+                                       Queue(new FlyCircle(self));
                                return NextActivity;
+                       }
 
                        // Don't turn until we've reached the cruise altitude
                        var desiredFacing = d.Yaw.Facing;
diff --git a/OpenRA.Mods.Common/Traits/Air/Aircraft.cs b/OpenRA.Mods.Common/Traits/Air/Aircraft.cs
index 74c596a5b8..a13ac8228d 100644
--- a/OpenRA.Mods.Common/Traits/Air/Aircraft.cs
+++ b/OpenRA.Mods.Common/Traits/Air/Aircraft.cs
@@ -818,7 +818,7 @@ public void ResolveOrder(Actor self, Order order)
                                self.SetTargetLine(target, Color.Green);
 
                                if (!Info.CanHover)
-                                       self.QueueActivity(order.Queued, new Fly(self, target));
+                                       self.QueueActivity(order.Queued, new Fly(self, target, true));
                                else
                                        self.QueueActivity(order.Queued, new HeliFly(self, target));
                        }

GraionDilach · 2018-12-18T13:01:15Z

mods/ra/rules/aircraft.yaml

@@ -115,13 +115,13 @@ MIG:
 		Speed: 223
 		RepulsionSpeed: 40
 		MaximumPitch: 56
+		ActionsOnIdle: ReturnToBase


No need to 🙄, it has ReturnOnIdle on bleed, afterall.

The 🙄 was that the complains can be fixed simply with changing these to FlyCircle if I'm reading this right. And it was aimed more towards @abcdefg30.

Were it so easy...

pchote · 2018-12-19T09:19:15Z

OpenRA.Mods.Common/Traits/Air/Aircraft.cs

@@ -66,12 +75,12 @@ public class AircraftInfo : ITraitInfo, IPositionableInfo, IFacingInfo, IMoveInf
 		[Desc("Can the actor hover in place mid-air? If not, then the actor will have to remain in motion (circle around).")]
 		public readonly bool CanHover = false;

+		[Desc("List of possible action to take when becoming idle in descending order of priority. Invalid entries (e.g. Hover when CanHover is false) will be skipped.")]


Am I right to assume that we are working towards removing the CanHover flag as soon as the activity merging is done? If not, we should merge Hover and Circle into a single Wait (maybe a different name) and use CanHover to determine the behaviour.

(maybe a different name)

How about HoldPosition?

Actually, this is a bit tricky:

We have one case - D2k carryalls - that (at least currently, can't say yet how it'll look after the activities have been merged) need CanHover to work, but also circle on idle.

As for removing it after the activities are merged, that might be tricky as well, since it's used for hovering attacks too, for example.

Not sure what the best approach would be here.

reaperrr · 2018-12-20T19:48:16Z

That makes Yaks unusable. 👎 to this PR if that is the intended behaviour.

Calm down, don't shoot me for finally trying to fix this buggy mess just because fixing it occasionally makes it hard to 100% replicate bleed's behavior on the first try.

pchote · 2018-12-20T22:44:34Z

Another idea we could consider - but i'm sure it would be controversial - would be to have aircraft return on idle by default after normal move orders, and loiter after force-move orders.

GraionDilach · 2018-12-26T00:52:38Z

Could this have an IdleSpeed to go along with IdleTurnSpeed?

reaperrr · 2018-12-26T22:06:55Z

I need to finish the bot modules first and we want a playtest soon after that, so bumping to Next+1.

ghost · 2019-04-01T01:38:38Z

Might be controversial but I would support it if an attack order would not be transformed into a RTB order when the plane is out of ammo. That they auto-RTB on attack when out of ammo is a significant burden for precise unit micro and only useful when you order them to attack something and then go afk. Instad they should just get a move order.

Another idea we could consider - but i'm sure it would be controversial - would be to have aircraft return on idle by default after normal move orders, and loiter after force-move orders.

I could live with that and it would make sense for me.

If you want that your valuable aircraft circle somewhere on the battlefield, the order should be explicitly given and the location be selected intentionally. You can leave your planes in this safe area without the need to babysit them.

If you are moving your planes around, you should however always babysit them because you are likely in an unknown / dangerous area. The idea that they auto-RTB on idle makes more sense to me than letting them circle there. I would like it if they auto-RTB'ed at a lower speed than their normal speed. It can punish players for a mistake they made, but the auto-circle-by-default encourages laziness.

Everything else than letting the plane fly circles already requires precise micro and causes high APM. I would be open for a discussion how we could make planes and helicopters more interesting in RA and give them better defined roles. That yaks currently primarily serve in a spotter role and are only secondarily used in a strike role makes the difference to helicopters quite meaningless and I think we're missing opportunities due to this.

As long as there is no consensus about what their actual role is, the proposed behavior for force move would be a fair compromise IMO that allows investigating how air combat can be made more interesting, prepares players for a new mechanic (potentially used for other mods or replacing the current behavior in the future) while nothing is taken away from anyone.

abcdefg30 · 2019-04-01T07:38:49Z

👍 to not issuing a RTB when out of ammo (AIs can still do that, players have hotkeys).
👎 to returning without force move. If I order a unit somewhere I expect it to go there and stay, not do something on its own.

ghost · 2019-04-01T08:00:02Z

I can see where you're coming from but would have favoured to defer the final decision over the point you're opposing to the playtest(s) and (let players) try it out first.

pchote · 2019-04-01T11:28:16Z

In any case, the auto-RTB behaviour should be available through a yaml flag for mods that want to recreate the proper C&C style aircraft behaviour.

pchote · 2019-05-23T17:58:51Z

Closing to help clean up the PR queue. We can reopen this once the prereqs are merged.

reaperrr added this to the Next Release milestone Dec 2, 2018

reaperrr added PR: Has dependencies PR: Is Dependency labels Dec 2, 2018

reaperrr force-pushed the air-ActionsOnIdle branch from c420e8d to 036564a Compare December 2, 2018 15:41

pchote added PR: Rebase me! and removed PR: Has dependencies labels Dec 4, 2018

reaperrr force-pushed the air-ActionsOnIdle branch from 036564a to af2bf63 Compare December 4, 2018 21:03

reaperrr removed the PR: Rebase me! label Dec 4, 2018

pchote reviewed Dec 4, 2018

View reviewed changes

OpenRA.Mods.Common/Activities/Air/FlyOffMap.cs Outdated Show resolved Hide resolved

OpenRA.Mods.Common/Traits/Air/Aircraft.cs Outdated Show resolved Hide resolved

pchote reviewed Dec 9, 2018

View reviewed changes

OpenRA.Mods.Common/Traits/Air/Aircraft.cs Outdated Show resolved Hide resolved

reaperrr force-pushed the air-ActionsOnIdle branch from ad28888 to e6d9998 Compare December 13, 2018 13:04

reaperrr added 2 commits December 13, 2018 14:10

Adapt official mods to ActionsOnIdle

8037eb1

Ran update rules on them first and then cleaned up.

Remove duplicate Aircraft from TS jumpjet husk

6e9875a

reaperrr force-pushed the air-ActionsOnIdle branch from e6d9998 to 6e9875a Compare December 13, 2018 13:11

reaperrr mentioned this pull request Dec 13, 2018

Merge and partially refactor aircraft activities #15803

Closed

abcdefg30 requested changes Dec 14, 2018

View reviewed changes

abcdefg30 added the PR: Changes requested label Dec 14, 2018

GraionDilach reviewed Dec 18, 2018

View reviewed changes

pchote reviewed Dec 19, 2018

View reviewed changes

reaperrr modified the milestones: Next Release, Next + 1 Dec 26, 2018

abcdefg30 added the PR: Rebase me! label Jan 4, 2019

reaperrr mentioned this pull request Jan 27, 2019

Milestone Roadmap #16131

Closed

reaperrr mentioned this pull request Mar 10, 2019

Merge HeliReturnToBase into ReturnToBase #16241

Merged

reaperrr mentioned this pull request Mar 22, 2019

Aircraft refactor #16338

Closed

1 task

reaperrr added the PR: Has dependencies label Mar 22, 2019

obrakmann mentioned this pull request Mar 26, 2019

Improve ResupplyAircraft #16321

Merged

pchote closed this May 23, 2019

pchote modified the milestones: Next Release, Future May 23, 2019

reaperrr deleted the air-ActionsOnIdle branch November 23, 2019 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Aircraft idle behavior #15873

Refactor Aircraft idle behavior #15873

reaperrr commented Dec 2, 2018

pchote commented Dec 4, 2018

reaperrr commented Dec 4, 2018

reaperrr commented Dec 9, 2018

reaperrr commented Dec 13, 2018

abcdefg30 left a comment

obrakmann commented Dec 17, 2018

abcdefg30 commented Dec 18, 2018

obrakmann commented Dec 18, 2018

GraionDilach Dec 18, 2018

reaperrr Dec 20, 2018

GraionDilach Dec 20, 2018

abcdefg30 Dec 20, 2018

pchote Dec 19, 2018

reaperrr Dec 20, 2018

reaperrr Dec 20, 2018

reaperrr commented Dec 20, 2018

pchote commented Dec 20, 2018

GraionDilach commented Dec 26, 2018

reaperrr commented Dec 26, 2018

ghost commented Apr 1, 2019

abcdefg30 commented Apr 1, 2019

ghost commented Apr 1, 2019

pchote commented Apr 1, 2019

pchote commented May 23, 2019

Refactor Aircraft idle behavior #15873

Refactor Aircraft idle behavior #15873

Conversation

reaperrr commented Dec 2, 2018

pchote commented Dec 4, 2018

reaperrr commented Dec 4, 2018

reaperrr commented Dec 9, 2018

reaperrr commented Dec 13, 2018

abcdefg30 left a comment

Choose a reason for hiding this comment

obrakmann commented Dec 17, 2018

abcdefg30 commented Dec 18, 2018

obrakmann commented Dec 18, 2018

GraionDilach Dec 18, 2018

Choose a reason for hiding this comment

reaperrr Dec 20, 2018

Choose a reason for hiding this comment

GraionDilach Dec 20, 2018

Choose a reason for hiding this comment

abcdefg30 Dec 20, 2018

Choose a reason for hiding this comment

pchote Dec 19, 2018

Choose a reason for hiding this comment

reaperrr Dec 20, 2018

Choose a reason for hiding this comment

reaperrr Dec 20, 2018

Choose a reason for hiding this comment

reaperrr commented Dec 20, 2018

pchote commented Dec 20, 2018

GraionDilach commented Dec 26, 2018

reaperrr commented Dec 26, 2018

ghost commented Apr 1, 2019

abcdefg30 commented Apr 1, 2019

ghost commented Apr 1, 2019

pchote commented Apr 1, 2019

pchote commented May 23, 2019