diff --git a/OpenRA.Game/Activities/Activity.cs b/OpenRA.Game/Activities/Activity.cs
index 9a7679ad23..e223321c78 100644
--- a/OpenRA.Game/Activities/Activity.cs
+++ b/OpenRA.Game/Activities/Activity.cs
@@ -9,42 +9,254 @@
*/
#endregion
+using System;
using System.Collections.Generic;
using System.Linq;
using OpenRA.Traits;
namespace OpenRA.Activities
{
+ public enum ActivityState { Queued, Active, Done, Canceled }
+
+ /*
+ * Activities are actions carried out by actors during each tick.
+ *
+ * Activities exist in a graph data structure built up amongst themselves. Each activity has a parent activity,
+ * 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:
+ *
+ * - Use "return NextActivity" at least once somewhere in the tick method.
+ * - Do not use "return new SomeActivity()" as that will break the graph. Queue the new activity and use "return NextActivity" instead.
+ * - 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 Activity NextActivity { get; set; }
+ public ActivityState State { get; private set; }
+
+ ///
+ /// Returns the top-most activity *from the point of view of the calling activity*. Note that the root activity
+ /// can and likely will have next activities of its own, which would in turn be the root for their children.
+ ///
+ public Activity RootActivity
+ {
+ get
+ {
+ var p = this;
+ while (p.ParentActivity != null)
+ p = p.ParentActivity;
+
+ return p;
+ }
+ }
+
+ Activity parentActivity;
+ public Activity ParentActivity
+ {
+ get
+ {
+ return parentActivity;
+ }
+
+ protected set
+ {
+ parentActivity = value;
+
+ var next = NextInQueue;
+ if (next != null)
+ next.ParentActivity = parentActivity;
+ }
+ }
+
+ Activity childActivity;
+ protected Activity ChildActivity
+ {
+ get
+ {
+ return childActivity != null && childActivity.State < ActivityState.Done ? childActivity : null;
+ }
+
+ set
+ {
+ if (value == this || value == ParentActivity || value == NextInQueue)
+ childActivity = null;
+ else
+ {
+ childActivity = value;
+
+ if (childActivity != null)
+ childActivity.ParentActivity = this;
+ }
+ }
+ }
+
+ Activity nextActivity;
+
+ ///
+ /// The getter will return either the next activity or, if there is none, the parent one.
+ ///
+ public virtual Activity NextActivity
+ {
+ get
+ {
+ return nextActivity != null ? nextActivity : ParentActivity;
+ }
+
+ set
+ {
+ if (value == this || value == ParentActivity || (value != null && value.ParentActivity == this))
+ nextActivity = null;
+ else
+ {
+ nextActivity = value;
+
+ if (nextActivity != null)
+ nextActivity.ParentActivity = ParentActivity;
+ }
+ }
+ }
+
+ ///
+ /// The getter will return the next activity on the same level _only_, in contrast to NextActivity.
+ /// Use this to check whether there are any follow-up activities queued.
+ ///
+ public Activity NextInQueue
+ {
+ get { return nextActivity; }
+ set { NextActivity = value; }
+ }
+
public bool IsInterruptible { get; protected set; }
- protected bool IsCanceled { get; private set; }
+ public bool IsCanceled { get { return State == ActivityState.Canceled; } }
public Activity()
{
IsInterruptible = true;
}
+ public Activity TickOuter(Actor self)
+ {
+ if (State == ActivityState.Done && Game.Settings.Debug.StrictActivityChecking)
+ throw new InvalidOperationException("Actor {0} attempted to tick activity {1} after it had already completed.".F(self, this.GetType()));
+
+ if (State == ActivityState.Queued)
+ {
+ OnFirstRun(self);
+ State = ActivityState.Active;
+ }
+
+ var ret = Tick(self);
+ if (ret == null || (ret != this && ret.ParentActivity != this))
+ {
+ // Make sure that the Parent's ChildActivity pointer is moved forwards as the child queue advances.
+ // The Child's ParentActivity will be set automatically during assignment.
+ if (ParentActivity != null && ParentActivity != ret)
+ ParentActivity.ChildActivity = ret;
+
+ if (State != ActivityState.Canceled)
+ State = ActivityState.Done;
+
+ OnLastRun(self);
+ }
+
+ return ret;
+ }
+
public abstract Activity Tick(Actor self);
+ ///
+ /// Runs once immediately before the first Tick() execution.
+ ///
+ protected virtual void OnFirstRun(Actor self) { }
+
+ ///
+ /// Runs once immediately after the last Tick() execution.
+ ///
+ protected virtual void OnLastRun(Actor self) { }
+
public virtual bool Cancel(Actor self)
{
if (!IsInterruptible)
return false;
- IsCanceled = true;
+ if (ChildActivity != null && !ChildActivity.Cancel(self))
+ return false;
+
+ State = ActivityState.Canceled;
NextActivity = null;
+ ChildActivity = null;
return true;
}
public virtual void Queue(Activity activity)
{
- if (NextActivity != null)
- NextActivity.Queue(activity);
+ if (NextInQueue != null)
+ NextInQueue.Queue(activity);
else
- NextActivity = activity;
+ NextInQueue = activity;
+ }
+
+ public virtual void QueueChild(Activity activity)
+ {
+ if (ChildActivity != null)
+ ChildActivity.Queue(activity);
+ else
+ ChildActivity = activity;
+ }
+
+ ///
+ /// Prints the activity tree, starting from the root or optionally from a given origin.
+ ///
+ /// Call this method from any place that's called during a tick, such as the Tick() method itself or
+ /// the Before(First|Last)Run() methods. The origin activity will be marked in the output.
+ ///
+ /// Activity from which to start traversing, and which to mark. If null, mark the calling activity, and start traversal from the root.
+ /// Initial level of indentation.
+ protected void PrintActivityTree(Activity origin = null, int level = 0)
+ {
+ if (origin == null)
+ RootActivity.PrintActivityTree(this);
+ else
+ {
+ Console.Write(new string(' ', level * 2));
+ if (origin == this)
+ Console.Write("*");
+
+ Console.WriteLine(this.GetType().ToString().Split('.').Last());
+
+ if (ChildActivity != null)
+ ChildActivity.PrintActivityTree(origin, level + 1);
+
+ if (NextInQueue != null)
+ NextInQueue.PrintActivityTree(origin, level);
+ }
}
public virtual IEnumerable GetTargets(Actor self)
@@ -53,6 +265,29 @@ namespace OpenRA.Activities
}
}
+ ///
+ /// In contrast to the base activity class, which is responsible for running its children itself,
+ /// composite activities rely on the actor's activity-running logic for their children.
+ ///
+ public abstract class CompositeActivity : Activity
+ {
+ ///
+ /// The getter will return the first non-null value of either child, next or parent activity, in that order, or ultimately null.
+ ///
+ public override Activity NextActivity
+ {
+ get
+ {
+ if (ChildActivity != null)
+ return ChildActivity;
+ else if (NextInQueue != null)
+ return NextInQueue;
+ else
+ return ParentActivity;
+ }
+ }
+ }
+
public static class ActivityExts
{
public static IEnumerable GetTargetQueue(this Actor self)
diff --git a/OpenRA.Game/Actor.cs b/OpenRA.Game/Actor.cs
index 950d4306c0..a12662d29e 100644
--- a/OpenRA.Game/Actor.cs
+++ b/OpenRA.Game/Actor.cs
@@ -203,13 +203,13 @@ namespace OpenRA
if (CurrentActivity == null)
CurrentActivity = nextActivity;
else
- CurrentActivity.Queue(nextActivity);
+ CurrentActivity.RootActivity.Queue(nextActivity);
}
public bool CancelActivity()
{
if (CurrentActivity != null)
- return CurrentActivity.Cancel(this);
+ return CurrentActivity.RootActivity.Cancel(this);
return true;
}
diff --git a/OpenRA.Game/Settings.cs b/OpenRA.Game/Settings.cs
index 9dc80aa7d2..9433a6b045 100644
--- a/OpenRA.Game/Settings.cs
+++ b/OpenRA.Game/Settings.cs
@@ -92,6 +92,7 @@ namespace OpenRA
public bool SanityCheckUnsyncedCode = false;
public int Samples = 25;
public bool IgnoreVersionMismatch = false;
+ public bool StrictActivityChecking = false;
public bool SendSystemInformation = true;
public int SystemInformationVersionPrompt = 0;
public string UUID = System.Guid.NewGuid().ToString();
diff --git a/OpenRA.Game/Traits/ActivityUtils.cs b/OpenRA.Game/Traits/ActivityUtils.cs
index 210461d050..685b26eaf2 100644
--- a/OpenRA.Game/Traits/ActivityUtils.cs
+++ b/OpenRA.Game/Traits/ActivityUtils.cs
@@ -34,7 +34,7 @@ namespace OpenRA.Traits
while (act != null)
{
var prev = act;
- act = act.Tick(self);
+ act = act.TickOuter(self);
var current = Stopwatch.GetTimestamp();
if (current - start > longTickThresholdInStopwatchTicks)
{
@@ -44,7 +44,7 @@ namespace OpenRA.Traits
else
start = current;
- if (prev == act)
+ if (act == prev || act == prev.ParentActivity)
break;
}
diff --git a/OpenRA.Mods.Common/Activities/Air/FlyAttack.cs b/OpenRA.Mods.Common/Activities/Air/FlyAttack.cs
index c42ef8d796..d8421075bd 100644
--- a/OpenRA.Mods.Common/Activities/Air/FlyAttack.cs
+++ b/OpenRA.Mods.Common/Activities/Air/FlyAttack.cs
@@ -23,7 +23,6 @@ namespace OpenRA.Mods.Common.Activities
readonly AttackPlane attackPlane;
readonly AmmoPool[] ammoPools;
- Activity inner;
int ticksUntilTurn;
public FlyAttack(Actor self, Target target)
@@ -47,34 +46,25 @@ namespace OpenRA.Mods.Common.Activities
if (attackPlane != null)
attackPlane.DoAttack(self, target);
- if (inner == null)
+ if (ChildActivity == null)
{
if (IsCanceled)
return NextActivity;
// TODO: This should fire each weapon at its maximum range
if (attackPlane != null && target.IsInRange(self.CenterPosition, attackPlane.Armaments.Select(a => a.Weapon.MinRange).Min()))
- inner = ActivityUtils.SequenceActivities(new FlyTimed(ticksUntilTurn, self), new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
+ ChildActivity = ActivityUtils.SequenceActivities(new FlyTimed(ticksUntilTurn, self), new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
else
- inner = ActivityUtils.SequenceActivities(new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
+ ChildActivity = ActivityUtils.SequenceActivities(new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
// HACK: This needs to be done in this round-about way because TakeOff doesn't behave as expected when it doesn't have a NextActivity.
if (self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length < aircraft.Info.MinAirborneAltitude)
- inner = ActivityUtils.SequenceActivities(new TakeOff(self), inner);
+ ChildActivity = ActivityUtils.SequenceActivities(new TakeOff(self), ChildActivity);
}
- inner = ActivityUtils.RunActivity(self, inner);
+ ActivityUtils.RunActivity(self, ChildActivity);
return this;
}
-
- public override bool Cancel(Actor self)
- {
- if (!IsCanceled && inner != null && !inner.Cancel(self))
- return false;
-
- // NextActivity must always be set to null:
- return base.Cancel(self);
- }
}
}
diff --git a/OpenRA.Mods.Common/Activities/Air/ResupplyAircraft.cs b/OpenRA.Mods.Common/Activities/Air/ResupplyAircraft.cs
index 01165caeca..9a92ef28b7 100644
--- a/OpenRA.Mods.Common/Activities/Air/ResupplyAircraft.cs
+++ b/OpenRA.Mods.Common/Activities/Air/ResupplyAircraft.cs
@@ -16,59 +16,40 @@ using OpenRA.Traits;
namespace OpenRA.Mods.Common.Activities
{
- public class ResupplyAircraft : Activity
+ public class ResupplyAircraft : CompositeActivity
{
- readonly Aircraft aircraft;
- Activity inner;
+ public ResupplyAircraft(Actor self) { }
- public ResupplyAircraft(Actor self)
+ protected override void OnFirstRun(Actor self)
{
- aircraft = self.Trait();
+ var aircraft = self.Trait();
+ var host = aircraft.GetActorBelow();
+
+ if (host == null)
+ return;
+
+ if (aircraft.IsPlane)
+ {
+ ChildActivity = ActivityUtils.SequenceActivities(
+ aircraft.GetResupplyActivities(host)
+ .Append(new AllowYieldingReservation(self))
+ .Append(new WaitFor(() => NextInQueue != null || aircraft.ReservedActor == null))
+ .ToArray());
+ }
+ else
+ {
+ // Helicopters should take off from their helipad immediately after resupplying.
+ // HACK: Append NextInQueue to TakeOff to avoid moving to the Rallypoint (if NextInQueue is non-null).
+ ChildActivity = ActivityUtils.SequenceActivities(
+ aircraft.GetResupplyActivities(host)
+ .Append(new AllowYieldingReservation(self))
+ .Append(new TakeOff(self)).Append(NextInQueue).ToArray());
+ }
}
public override Activity Tick(Actor self)
{
- if (IsCanceled)
- return NextActivity;
-
- if (inner == null)
- {
- var host = aircraft.GetActorBelow();
-
- if (host == null)
- return NextActivity;
-
- if (aircraft.IsPlane)
- {
- inner = ActivityUtils.SequenceActivities(
- aircraft.GetResupplyActivities(host)
- .Append(new AllowYieldingReservation(self))
- .Append(new WaitFor(() => NextActivity != null || aircraft.ReservedActor == null))
- .ToArray());
- }
- else
- {
- // Helicopters should take off from their helipad immediately after resupplying.
- // HACK: Append NextActivity to TakeOff to avoid moving to the Rallypoint (if NextActivity is non-null).
- inner = ActivityUtils.SequenceActivities(
- aircraft.GetResupplyActivities(host)
- .Append(new AllowYieldingReservation(self))
- .Append(new TakeOff(self)).Append(NextActivity).ToArray());
- }
- }
- else
- inner = ActivityUtils.RunActivity(self, inner);
-
- // The inner == NextActivity check is needed here because of the TakeOff issue mentioned in the comment above.
- return inner == null || inner == NextActivity ? NextActivity : this;
- }
-
- public override bool Cancel(Actor self)
- {
- if (!IsCanceled && inner != null && !inner.Cancel(self))
- return false;
-
- return base.Cancel(self);
+ return NextActivity;
}
}
}
diff --git a/OpenRA.Mods.Common/Activities/Air/TakeOff.cs b/OpenRA.Mods.Common/Activities/Air/TakeOff.cs
index 4f0cfe4690..c271d565aa 100644
--- a/OpenRA.Mods.Common/Activities/Air/TakeOff.cs
+++ b/OpenRA.Mods.Common/Activities/Air/TakeOff.cs
@@ -37,10 +37,10 @@ namespace OpenRA.Mods.Common.Activities
var destination = rp != null ? rp.Location :
(hasHost ? self.World.Map.CellContaining(host.CenterPosition) : self.Location);
- if (NextActivity == null)
+ if (NextInQueue == null)
return new AttackMoveActivity(self, move.MoveTo(destination, 1));
else
- return NextActivity;
+ return NextInQueue;
}
}
}
diff --git a/OpenRA.Mods.Common/Activities/DeliverResources.cs b/OpenRA.Mods.Common/Activities/DeliverResources.cs
index d3b633d942..8ec4865794 100644
--- a/OpenRA.Mods.Common/Activities/DeliverResources.cs
+++ b/OpenRA.Mods.Common/Activities/DeliverResources.cs
@@ -35,8 +35,8 @@ namespace OpenRA.Mods.Common.Activities
public override Activity Tick(Actor self)
{
- if (NextActivity != null)
- return NextActivity;
+ if (NextInQueue != null)
+ return NextInQueue;
// Find the nearest best refinery if not explicitly ordered to a specific refinery:
if (harv.OwnerLinkedProc == null || !harv.OwnerLinkedProc.IsInWorld)
diff --git a/OpenRA.Mods.Common/Activities/FindResources.cs b/OpenRA.Mods.Common/Activities/FindResources.cs
index 7e99d9bb99..443dc167be 100644
--- a/OpenRA.Mods.Common/Activities/FindResources.cs
+++ b/OpenRA.Mods.Common/Activities/FindResources.cs
@@ -51,9 +51,12 @@ namespace OpenRA.Mods.Common.Activities
public override Activity Tick(Actor self)
{
- if (IsCanceled || NextActivity != null)
+ if (IsCanceled)
return NextActivity;
+ if (NextInQueue != null)
+ return NextInQueue;
+
var deliver = new DeliverResources(self);
if (harv.IsFull)
@@ -81,8 +84,8 @@ namespace OpenRA.Mods.Common.Activities
var randFrames = self.World.SharedRandom.Next(100, 175);
// Avoid creating an activity cycle
- var next = NextActivity;
- NextActivity = null;
+ var next = NextInQueue;
+ NextInQueue = null;
return ActivityUtils.SequenceActivities(next, new Wait(randFrames), this);
}
else