From 705795abdeaabf6c8f852bcd1a3eb23b0445fced Mon Sep 17 00:00:00 2001 From: tovl Date: Sat, 9 Mar 2019 15:49:12 +0100 Subject: [PATCH] Activity.Cancel returns void instead of bool. --- OpenRA.Game/Activities/Activity.cs | 24 +++---------------- OpenRA.Game/Actor.cs | 6 ++--- OpenRA.Mods.Common/Activities/DeliverUnit.cs | 4 ++-- OpenRA.Mods.Common/Activities/Enter.cs | 4 ++-- .../Activities/EnterTransport.cs | 4 ++-- .../Activities/HarvesterDockSequence.cs | 4 ++-- .../Activities/Move/AttackMoveActivity.cs | 4 ++-- .../Activities/Move/MoveAdjacentTo.cs | 4 ++-- OpenRA.Mods.Common/Activities/PickupUnit.cs | 4 ++-- OpenRA.Mods.Common/Activities/Wait.cs | 24 +++++-------------- .../Activities/WaitForTransport.cs | 4 ++-- OpenRA.Mods.Common/Scripting/CallLuaFunc.cs | 4 ++-- .../Traits/TransformCrusherOnCrush.cs | 4 ++-- 13 files changed, 31 insertions(+), 63 deletions(-) diff --git a/OpenRA.Game/Activities/Activity.cs b/OpenRA.Game/Activities/Activity.cs index 36f9d8a847..620e148f0a 100644 --- a/OpenRA.Game/Activities/Activity.cs +++ b/OpenRA.Game/Activities/Activity.cs @@ -25,11 +25,6 @@ namespace OpenRA.Activities * optionally child activities, and usually a next activity. An actor's CurrentActivity is a pointer into that graph * and moves through it as activities run. * - * There are two kinds of activities, the base activity and composite activities. They differ in the way their children - * are run: while a base activity is responsible for running its children itself, a composite activity relies on the actor's - * activity-running code. Therefore, the actor's CurrentActivity stays on the base activity while it runs its children. With - * composite activities however, the CurrentActivity moves through the list of children as they run. - * * * Things to be aware of when writing activities: * @@ -38,24 +33,13 @@ namespace OpenRA.Activities * - Do not "reuse" (with "SequenceActivities", for example) activity objects that have already finished running. * Queue a new instance instead. * - Avoid calling actor.CancelActivity(). It is almost always a bug. Call activity.Cancel() instead. - * - A composite activity will run at least twice. The first time when it returns its children, - * the second time when its last child returns its Parent. * - Do not return the Parent explicitly unless you have an extremly good reason. "return NextActivity" * will do the right thing in all circumstances. * - You do not need to care about the ChildActivity pointer advancing through the list of children, * the activity code already takes care of that. * - If you want to check whether there are any follow-up activities queued, check against "NextInQueue" * in favour of "NextActivity" to avoid checking against the Parent activity. - * - * - * Guide when to use which kind of activity: - * - * - The activity does not have any children -> base activity - * - The activity needs to run preparatory steps during each tick before its children can be run -> base activity - * - The activity or the actor is left in a bogus state when one of the child activities is canceled -> base activity - * - The activity's children are self-contained and can run independently of the parent -> composite activity - * - The activity does not have any or little logic of its own, but is just composed of sub-steps -> composite activity - */ + */ public abstract class Activity { public ActivityState State { get; private set; } @@ -217,20 +201,18 @@ namespace OpenRA.Activities OnActorDispose(self); } - public virtual bool Cancel(Actor self, bool keepQueue = false) + public virtual void Cancel(Actor self, bool keepQueue = false) { if (!keepQueue) NextInQueue = null; if (!IsInterruptible) - return false; + return; if (ChildActivity != null) ChildActivity.Cancel(self); State = ActivityState.Canceling; - - return true; } public virtual void Queue(Actor self, Activity activity, bool pretick = false) diff --git a/OpenRA.Game/Actor.cs b/OpenRA.Game/Actor.cs index 3c46a71617..9404259f63 100644 --- a/OpenRA.Game/Actor.cs +++ b/OpenRA.Game/Actor.cs @@ -225,12 +225,10 @@ namespace OpenRA CurrentActivity.RootActivity.Queue(this, nextActivity); } - public bool CancelActivity() + public void CancelActivity() { if (CurrentActivity != null) - return CurrentActivity.RootActivity.Cancel(this); - - return true; + CurrentActivity.RootActivity.Cancel(this); } public override int GetHashCode() diff --git a/OpenRA.Mods.Common/Activities/DeliverUnit.cs b/OpenRA.Mods.Common/Activities/DeliverUnit.cs index 2f12a1f5c5..d13c9768cd 100644 --- a/OpenRA.Mods.Common/Activities/DeliverUnit.cs +++ b/OpenRA.Mods.Common/Activities/DeliverUnit.cs @@ -206,12 +206,12 @@ namespace OpenRA.Mods.Common.Activities }); } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && innerActivity != null) innerActivity.Cancel(self); - return base.Cancel(self); + base.Cancel(self); } } } diff --git a/OpenRA.Mods.Common/Activities/Enter.cs b/OpenRA.Mods.Common/Activities/Enter.cs index 15fca2a977..6e4a1ba86e 100644 --- a/OpenRA.Mods.Common/Activities/Enter.cs +++ b/OpenRA.Mods.Common/Activities/Enter.cs @@ -160,14 +160,14 @@ namespace OpenRA.Mods.Common.Activities return this; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { OnCancel(self); if (!IsCanceling && moveActivity != null) moveActivity.Cancel(self); - return base.Cancel(self, keepQueue); + base.Cancel(self, keepQueue); } } } diff --git a/OpenRA.Mods.Common/Activities/EnterTransport.cs b/OpenRA.Mods.Common/Activities/EnterTransport.cs index 6648b3728a..dc4e09788b 100644 --- a/OpenRA.Mods.Common/Activities/EnterTransport.cs +++ b/OpenRA.Mods.Common/Activities/EnterTransport.cs @@ -133,12 +133,12 @@ namespace OpenRA.Mods.Common.Activities return NextActivity; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && enterTransport != null) enterTransport.Cancel(self); - return base.Cancel(self, keepQueue); + base.Cancel(self, keepQueue); } } } diff --git a/OpenRA.Mods.Common/Activities/HarvesterDockSequence.cs b/OpenRA.Mods.Common/Activities/HarvesterDockSequence.cs index fcfe92ec94..78fdee3466 100644 --- a/OpenRA.Mods.Common/Activities/HarvesterDockSequence.cs +++ b/OpenRA.Mods.Common/Activities/HarvesterDockSequence.cs @@ -81,10 +81,10 @@ namespace OpenRA.Mods.Common.Activities throw new InvalidOperationException("Invalid harvester dock state"); } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { dockingState = DockingState.Undock; - return base.Cancel(self); + base.Cancel(self); } public override IEnumerable GetTargets(Actor self) diff --git a/OpenRA.Mods.Common/Activities/Move/AttackMoveActivity.cs b/OpenRA.Mods.Common/Activities/Move/AttackMoveActivity.cs index 7c0fa2b342..538761e7b1 100644 --- a/OpenRA.Mods.Common/Activities/Move/AttackMoveActivity.cs +++ b/OpenRA.Mods.Common/Activities/Move/AttackMoveActivity.cs @@ -46,12 +46,12 @@ namespace OpenRA.Mods.Common.Activities return this; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && inner != null) inner.Cancel(self); - return base.Cancel(self, keepQueue); + base.Cancel(self, keepQueue); } public override IEnumerable GetTargets(Actor self) diff --git a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs index 19f68b605a..ff6c9ab728 100644 --- a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs +++ b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs @@ -167,12 +167,12 @@ namespace OpenRA.Mods.Common.Activities return Target.None; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && inner != null) inner.Cancel(self); - return base.Cancel(self); + base.Cancel(self); } } } diff --git a/OpenRA.Mods.Common/Activities/PickupUnit.cs b/OpenRA.Mods.Common/Activities/PickupUnit.cs index 385bf49667..42c0a2f0b7 100644 --- a/OpenRA.Mods.Common/Activities/PickupUnit.cs +++ b/OpenRA.Mods.Common/Activities/PickupUnit.cs @@ -159,12 +159,12 @@ namespace OpenRA.Mods.Common.Activities }); } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && innerActivity != null) innerActivity.Cancel(self); - return base.Cancel(self, keepQueue); + base.Cancel(self, keepQueue); } } } diff --git a/OpenRA.Mods.Common/Activities/Wait.cs b/OpenRA.Mods.Common/Activities/Wait.cs index d6cabb492b..dde648dbb3 100644 --- a/OpenRA.Mods.Common/Activities/Wait.cs +++ b/OpenRA.Mods.Common/Activities/Wait.cs @@ -27,17 +27,11 @@ namespace OpenRA.Mods.Common.Activities public override Activity Tick(Actor self) { + if (IsCanceling) + return NextActivity; + return (remainingTicks-- == 0) ? NextActivity : this; } - - public override bool Cancel(Actor self, bool keepQueue = false) - { - if (!base.Cancel(self, keepQueue)) - return false; - - remainingTicks = 0; - return true; - } } public class WaitFor : Activity @@ -53,16 +47,10 @@ namespace OpenRA.Mods.Common.Activities public override Activity Tick(Actor self) { + if (IsCanceling) + return NextActivity; + return (f == null || f()) ? NextActivity : this; } - - public override bool Cancel(Actor self, bool keepQueue = false) - { - if (!base.Cancel(self, keepQueue)) - return false; - - f = null; - return true; - } } } diff --git a/OpenRA.Mods.Common/Activities/WaitForTransport.cs b/OpenRA.Mods.Common/Activities/WaitForTransport.cs index c7cda99eb9..f1e4d6be8d 100644 --- a/OpenRA.Mods.Common/Activities/WaitForTransport.cs +++ b/OpenRA.Mods.Common/Activities/WaitForTransport.cs @@ -41,12 +41,12 @@ namespace OpenRA.Mods.Common.Activities return this; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { if (!IsCanceling && inner != null) inner.Cancel(self); - return base.Cancel(self, keepQueue); + base.Cancel(self, keepQueue); } } } diff --git a/OpenRA.Mods.Common/Scripting/CallLuaFunc.cs b/OpenRA.Mods.Common/Scripting/CallLuaFunc.cs index 39df73eedc..074d8c1064 100644 --- a/OpenRA.Mods.Common/Scripting/CallLuaFunc.cs +++ b/OpenRA.Mods.Common/Scripting/CallLuaFunc.cs @@ -43,11 +43,11 @@ namespace OpenRA.Mods.Common.Activities return NextActivity; } - public override bool Cancel(Actor self, bool keepQueue = false) + public override void Cancel(Actor self, bool keepQueue = false) { base.Cancel(self, keepQueue); Dispose(); - return true; + return; } public void Dispose() diff --git a/OpenRA.Mods.Common/Traits/TransformCrusherOnCrush.cs b/OpenRA.Mods.Common/Traits/TransformCrusherOnCrush.cs index 624cf78122..d7a8fcade7 100644 --- a/OpenRA.Mods.Common/Traits/TransformCrusherOnCrush.cs +++ b/OpenRA.Mods.Common/Traits/TransformCrusherOnCrush.cs @@ -51,8 +51,8 @@ namespace OpenRA.Mods.Common.Traits transform.Facing = facing.Facing; transform.SkipMakeAnims = info.SkipMakeAnims; - if (crusher.CancelActivity()) - crusher.QueueActivity(transform); + crusher.CancelActivity(); + crusher.QueueActivity(transform); } } }