Simplify Activity class

After the removal of the CompositeActivity class, all the supporting
code that made it work can be removed as well.
This commit is contained in:
Oliver Brakmann
2019-03-26 03:11:03 +01:00
committed by Paul Chote
parent 19977bb7da
commit b4fd7331b2
6 changed files with 25 additions and 127 deletions

View File

@@ -19,122 +19,26 @@ namespace OpenRA.Activities
public enum ActivityState { Queued, Active, Canceling, Done } public enum ActivityState { Queued, Active, Canceling, Done }
/* /*
* 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.
*
*
* Things to be aware of when writing activities: * Things to be aware of when writing activities:
* *
* - Use "return NextActivity" at least once somewhere in the tick method. * - 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 use "return new SomeActivity()" as that will break the queue. Queue the new activity and use "return NextActivity" instead.
* - Do not "reuse" (with "SequenceActivities", for example) activity objects that have already finished running. * - Do not "reuse" (with "SequenceActivities", for example) activity objects that have already started running.
* Queue a new instance instead. * Queue a new instance instead.
* - Avoid calling actor.CancelActivity(). It is almost always a bug. Call activity.Cancel() instead. * - Avoid calling actor.CancelActivity(). It is almost always a bug. Call activity.Cancel() instead.
* - 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.
*/ */
public abstract class Activity public abstract class Activity
{ {
public ActivityState State { get; private set; } public ActivityState State { get; private set; }
/// <summary>
/// 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.
/// </summary>
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; Activity childActivity;
protected Activity ChildActivity protected Activity ChildActivity
{ {
get get { return childActivity != null && childActivity.State < ActivityState.Done ? childActivity : null; }
{ set { childActivity = value; }
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; public Activity NextActivity { get; protected set; }
/// <summary>
/// The getter will return either the next activity or, if there is none, the parent one.
/// </summary>
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;
}
}
}
/// <summary>
/// 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.
/// </summary>
public Activity NextInQueue
{
get { return nextActivity; }
set { NextActivity = value; }
}
public bool IsInterruptible { get; protected set; } public bool IsInterruptible { get; protected set; }
public bool IsCanceling { get { return State == ActivityState.Canceling; } } public bool IsCanceling { get { return State == ActivityState.Canceling; } }
@@ -156,15 +60,9 @@ namespace OpenRA.Activities
} }
var ret = Tick(self); var ret = Tick(self);
if (ret == null || (ret != this && ret.ParentActivity != this)) if (ret != 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;
State = ActivityState.Done; State = ActivityState.Done;
OnLastRun(self); OnLastRun(self);
} }
@@ -204,7 +102,7 @@ namespace OpenRA.Activities
public virtual void Cancel(Actor self, bool keepQueue = false) public virtual void Cancel(Actor self, bool keepQueue = false)
{ {
if (!keepQueue) if (!keepQueue)
NextInQueue = null; NextActivity = null;
if (!IsInterruptible) if (!IsInterruptible)
return; return;
@@ -217,10 +115,10 @@ namespace OpenRA.Activities
public virtual void Queue(Actor self, Activity activity) public virtual void Queue(Actor self, Activity activity)
{ {
if (NextInQueue != null) if (NextActivity != null)
NextInQueue.Queue(self, activity); NextActivity.Queue(self, activity);
else else
NextInQueue = activity; NextActivity = activity;
} }
public virtual void QueueChild(Actor self, Activity activity, bool pretick = false) public virtual void QueueChild(Actor self, Activity activity, bool pretick = false)
@@ -232,17 +130,17 @@ namespace OpenRA.Activities
} }
/// <summary> /// <summary>
/// Prints the activity tree, starting from the root or optionally from a given origin. /// Prints the activity tree, starting from the top 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 /// 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. /// the Before(First|Last)Run() methods. The origin activity will be marked in the output.
/// </summary> /// </summary>
/// <param name="origin">Activity from which to start traversing, and which to mark. If null, mark the calling activity, and start traversal from the root.</param> /// <param name="origin">Activity from which to start traversing, and which to mark. If null, mark the calling activity, and start traversal from the top.</param>
/// <param name="level">Initial level of indentation.</param> /// <param name="level">Initial level of indentation.</param>
protected void PrintActivityTree(Actor self, Activity origin = null, int level = 0) protected void PrintActivityTree(Actor self, Activity origin = null, int level = 0)
{ {
if (origin == null) if (origin == null)
RootActivity.PrintActivityTree(self, this); self.CurrentActivity.PrintActivityTree(self, this);
else else
{ {
Console.Write(new string(' ', level * 2)); Console.Write(new string(' ', level * 2));
@@ -254,8 +152,8 @@ namespace OpenRA.Activities
if (ChildActivity != null) if (ChildActivity != null)
ChildActivity.PrintActivityTree(self, origin, level + 1); ChildActivity.PrintActivityTree(self, origin, level + 1);
if (NextInQueue != null) if (NextActivity != null)
NextInQueue.PrintActivityTree(self, origin, level); NextActivity.PrintActivityTree(self, origin, level);
} }
} }

View File

@@ -222,13 +222,13 @@ namespace OpenRA
if (CurrentActivity == null) if (CurrentActivity == null)
CurrentActivity = nextActivity; CurrentActivity = nextActivity;
else else
CurrentActivity.RootActivity.Queue(this, nextActivity); CurrentActivity.Queue(this, nextActivity);
} }
public void CancelActivity() public void CancelActivity()
{ {
if (CurrentActivity != null) if (CurrentActivity != null)
CurrentActivity.RootActivity.Cancel(this); CurrentActivity.Cancel(this);
} }
public override int GetHashCode() public override int GetHashCode()
@@ -281,7 +281,7 @@ namespace OpenRA
// If CurrentActivity isn't null, run OnActorDisposeOuter in case some cleanups are needed. // If CurrentActivity isn't null, run OnActorDisposeOuter in case some cleanups are needed.
// This should be done before the FrameEndTask to avoid dependency issues. // This should be done before the FrameEndTask to avoid dependency issues.
if (CurrentActivity != null) if (CurrentActivity != null)
CurrentActivity.RootActivity.OnActorDisposeOuter(this); CurrentActivity.OnActorDisposeOuter(this);
// Allow traits/activities to prevent a race condition when they depend on disposing the actor (e.g. Transforms) // Allow traits/activities to prevent a race condition when they depend on disposing the actor (e.g. Transforms)
WillDispose = true; WillDispose = true;

View File

@@ -44,7 +44,7 @@ namespace OpenRA.Traits
else else
start = current; start = current;
if (act == prev || act == prev.ParentActivity) if (act == prev)
break; break;
} }

View File

@@ -32,13 +32,13 @@ namespace OpenRA.Mods.Common.Activities
{ {
ChildActivity = ActivityUtils.SequenceActivities(self, aircraft.GetResupplyActivities(host).ToArray()); ChildActivity = ActivityUtils.SequenceActivities(self, aircraft.GetResupplyActivities(host).ToArray());
QueueChild(self, new AllowYieldingReservation(self)); QueueChild(self, new AllowYieldingReservation(self));
QueueChild(self, new WaitFor(() => NextInQueue != null || aircraft.ReservedActor == null)); QueueChild(self, new WaitFor(() => NextActivity != null || aircraft.ReservedActor == null));
} }
else else
{ {
ChildActivity = ActivityUtils.SequenceActivities(self, aircraft.GetResupplyActivities(host).ToArray()); ChildActivity = ActivityUtils.SequenceActivities(self, aircraft.GetResupplyActivities(host).ToArray());
QueueChild(self, new AllowYieldingReservation(self)); QueueChild(self, new AllowYieldingReservation(self));
QueueChild(self, new TakeOff(self, (a, b, c) => NextInQueue == null && b.NextInQueue == null)); QueueChild(self, new TakeOff(self, (a, b, c) => NextActivity == null && b.NextActivity == null));
} }
} }

View File

@@ -35,8 +35,8 @@ namespace OpenRA.Mods.Common.Activities
public override Activity Tick(Actor self) public override Activity Tick(Actor self)
{ {
if (NextInQueue != null) if (NextActivity != null)
return NextInQueue; return NextActivity;
// Find the nearest best refinery if not explicitly ordered to a specific refinery: // Find the nearest best refinery if not explicitly ordered to a specific refinery:
if (harv.OwnerLinkedProc == null || !harv.OwnerLinkedProc.IsInWorld) if (harv.OwnerLinkedProc == null || !harv.OwnerLinkedProc.IsInWorld)

View File

@@ -80,8 +80,8 @@ namespace OpenRA.Mods.Common.Activities
var randFrames = self.World.SharedRandom.Next(100, 175); var randFrames = self.World.SharedRandom.Next(100, 175);
// Avoid creating an activity cycle // Avoid creating an activity cycle
var next = NextInQueue; var next = NextActivity;
NextInQueue = null; NextActivity = null;
return ActivityUtils.SequenceActivities(self, next, new Wait(randFrames), this); return ActivityUtils.SequenceActivities(self, next, new Wait(randFrames), this);
} }
else else